Re: [PATCH v2 8/8] virtio-gpu: Add gl_flushed callback
Hi,
> -if (!cmd->finished) {
> +if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
> virtio_gpu_ctrl_response_nodata(g, cmd, cmd->error ? cmd->error :
> VIRTIO_GPU_RESP_OK_NODATA);
> }
My idea would be more along the lines of ...
if (!cmd->finished) {
if (renderer_blocked) {
g->pending_completion = cmd;
} else {
virtio_gpu_ctrl_response_nodata(...)
}
}
Then, when resuming processing after unblock check pending_completion
and call virtio_gpu_ctrl_response_nodata if needed.
Workflow:
virtio_gpu_simple_process_cmd()
-> virtio_gpu_resource_flush()
-> dpy_gfx_update()
-> gd_gl_area_update()
call graphic_hw_gl_block(true), create fence.
virtio_gpu_simple_process_cmd()
-> will see renderer_blocked and delays RESOURCE_FLUSH completion.
Then, when the fence is ready, gtk will:
- call graphic_hw_gl_block(false)
- call graphic_hw_gl_flush()
-> virtio-gpu resumes processing the cmd queue.
When you use the existing block/unblock functionality the fence can be a
gtk internal detail, virtio-gpu doesn't need to know that gtk uses a
fence to wait for the moment when it can unblock virtio queue processing
(the egl fence helpers still make sense).
take care,
Gerd
Re: [PATCH v2 2/3] target/ppc: divided mmu_helper.c in 2 files
On Thu, Jun 10, 2021 at 01:46:47PM -0300, Lucas Mateus Castro (alqotel) wrote:
> Moved functions in mmu_helper.c that should be compiled in build to
"should be compiled in build" is not very clear to me. What's the
distinction between both the files.
> mmu_common.c, moved declaration of functions that both files use to
> cpu.h and moved struct declarations and inline functions needed by
> both to target/ppc/internal.h. Updated meson.build to compile the
> new file. ppc6xx_tlb_getnum is not an inline function anymore.
Overall this looks reasonable. I think there's quite a lot you put
into mmu_common.c that can TCG-only, but it's reasonable to delay the
cleanups that will allow that to happen until further down the track.
>
> Signed-off-by: Lucas Mateus Castro (alqotel)
> ---
> target/ppc/cpu.h| 21 +
> target/ppc/internal.h | 26 +
> target/ppc/meson.build |6 +-
> target/ppc/mmu_common.c | 1623 ++
> target/ppc/mmu_helper.c | 1814 +++
> 5 files changed, 1777 insertions(+), 1713 deletions(-)
> create mode 100644 target/ppc/mmu_common.c
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index b4de0db7ff..0804acc6f2 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1330,6 +1330,27 @@ void store_booke_tsr(CPUPPCState *env, target_ulong
> val);
> void ppc_tlb_invalidate_all(CPUPPCState *env);
> void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
> void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
> +
> +typedef struct mmu_ctx_t mmu_ctx_t;
> +int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
> +hwaddr *raddrp,
> +target_ulong address, uint32_t pid, int ext,
> +int i);
> +int get_physical_address_wtlb(CPUPPCState *env, mmu_ctx_t *ctx,
> + target_ulong eaddr,
> + MMUAccessType access_type, int type,
> + int mmu_idx);
> +hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
> +ppcmas_tlb_t *tlb);
> +int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
> +hwaddr *raddrp, target_ulong address,
> +uint32_t pid);
> +int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
> +target_ulong eaddr, MMUAccessType
> access_type,
> +int type);
> +int ppc6xx_tlb_getnum(CPUPPCState *env, target_ulong eaddr,
> +int way, int is_code);
> +
> #endif
> #endif
>
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index f1fd3c8d04..ec21a1fbfd 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -245,4 +245,30 @@ static inline int prot_for_access_type(MMUAccessType
> access_type)
> g_assert_not_reached();
> }
>
> +/* Context used internally during MMU translations */
> +
> +struct mmu_ctx_t {
> +hwaddr raddr; /* Real address */
> +hwaddr eaddr; /* Effective address */
> +int prot; /* Protection bits */
> +hwaddr hash[2];/* Pagetable hash values */
> +target_ulong ptem; /* Virtual segment ID | API */
> +int key; /* Access key*/
> +int nx;/* Non-execute area */
> +};
> +
> +/* Common routines used by software and hardware TLBs emulation */
> +static inline int pte_is_valid(target_ulong pte0)
> +{
> +return pte0 & 0x8000 ? 1 : 0;
> +}
> +
> +static inline void pte_invalidate(target_ulong *pte0)
> +{
> +*pte0 &= ~0x8000;
> +}
> +
> +#define PTE_PTEM_MASK 0x7FBF
> +#define PTE_CHECK_MASK (TARGET_PAGE_MASK | 0x7B)
> +
> #endif /* PPC_INTERNAL_H */
> diff --git a/target/ppc/meson.build b/target/ppc/meson.build
> index a4f18ff414..03933b666f 100644
> --- a/target/ppc/meson.build
> +++ b/target/ppc/meson.build
> @@ -37,10 +37,12 @@ ppc_softmmu_ss.add(files(
>'arch_dump.c',
>'machine.c',
>'mmu-hash32.c',
> - 'mmu_helper.c',
> + 'mmu_common.c',
>'monitor.c',
> ))
> -ppc_softmmu_ss.add(when: 'CONFIG_TCG', if_false: files(
> +ppc_softmmu_ss.add(when: 'CONFIG_TCG', if_true: files(
> + 'mmu_helper.c',
> +), if_false: files(
>'tcg-stub.c'
> ))
>
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> new file mode 100644
> index 00..93e7b8f955
> --- /dev/null
> +++ b/target/ppc/mmu_common.c
> @@ -0,0 +1,1623 @@
> +/*
> + * PowerPC CPU initialization for qemu.
> + *
> + * Copyright (C) 2021 Instituto de Pesquisas Eldorado (eldorado.org.br)
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free
Re: [PATCH v2 1/3] target/ppc: Turn ppc_tlb_invalid_all in a noop
On Thu, Jun 10, 2021 at 01:46:46PM -0300, Lucas Mateus Castro (alqotel) wrote:
> The function ppc_tlb_invalid_all is now a no op when compiling without TCG.
>
> Signed-off-by: Lucas Mateus Castro (alqotel)
Hm, I think I suggested making ppc_tlb_invalidate_all() a stub, rather
than removing the call from !TCG code. But looking at this again, I
think that was a mistake. I think it makes more sense to suppress
this on the caller side, and make this a TCG only function.
> ---
> target/ppc/mmu_helper.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 1ecb36e85a..e7ba39c9e1 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -825,6 +825,7 @@ static int mmubooke_get_physical_address(CPUPPCState
> *env, mmu_ctx_t *ctx,
> return ret;
> }
>
> +#ifdef CONFIG_TCG
> static void booke206_flush_tlb(CPUPPCState *env, int flags,
> const int check_iprot)
> {
> @@ -846,6 +847,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int
> flags,
>
> tlb_flush(env_cpu(env));
> }
> +#endif
>
> static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
> ppcmas_tlb_t *tlb)
> @@ -1956,6 +1958,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t
> nr, target_ulong value)
> /* TLB management */
> void ppc_tlb_invalidate_all(CPUPPCState *env)
> {
> +#ifdef CONFIG_TCG
> #if defined(TARGET_PPC64)
> if (mmu_is_64bit(env->mmu_model)) {
> env->tlb_need_flush = 0;
> @@ -1994,6 +1997,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
> cpu_abort(env_cpu(env), "Unknown MMU model %x\n", env->mmu_model);
> break;
> }
> +#endif
> }
>
> #ifdef CONFIG_TCG
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [RFC PATCH 3/8] spapr_numa.c: wait for CAS before writing rtas DT
On Mon, Jun 14, 2021 at 10:33:04PM -0300, Daniel Henrique Barboza wrote:
> spapr_numa_write_rtas_dt() is called from spapr_dt_rtas(), which in
> turned is called by spapr_build_fdt(). spapr_build_fdt() is called in
> two places: spapr_machine_reset() and do_client_architecture_support().
> When called in machine_reset() we're writing RTAS nodes with NUMA
> artifacts without going through CAS first.
>
> This is not an issue because we always write the same thing in DT, since
> we support just FORM1 NUMA affinity. With the upcoming FORM2 support,
> we're now reliant on guest choice to decide what to write.
>
> Instead of taking a guess (e.g. default to FORM1, switch to FORM2 if
> guest chooses it), postpone the writing of
> ibm,associativity-reference-points and ibm,max-associativity-domains
> until we're sure what was negotiated with the guest.
>
> Signed-off-by: Daniel Henrique Barboza
I think it makes sense to fold this in with 1/8 moving the calculation
itself until after CAS.
This does make a (theoretical) functional change - it means that NUMA
information is not available before CAS, which it was before. I think
that's very unlikely to break anything, but I wonder if we should make
it dependent on the machine version just to be safe.
> ---
> hw/ppc/spapr_numa.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index 04a86f9b5b..e1a7f80076 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -379,6 +379,10 @@ static void
> spapr_numa_FORM1_write_rtas_dt(SpaprMachineState *spapr,
> */
> void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas)
> {
> +if (spapr_ovec_empty(spapr->ov5_cas)) {
> +return;
> +}
> +
> spapr_numa_FORM1_write_rtas_dt(spapr, fdt, rtas);
> }
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v2 2/8] ui/egl: Add egl helpers to help with synchronization
Hi,
> +void egl_dmabuf_create_fence(QemuDmaBuf *dmabuf)
> +{
> +if (dmabuf->sync) {
> +dmabuf->fence_fd = eglDupNativeFenceFDANDROID(qemu_egl_display,
> + dmabuf->sync);
> +eglDestroySyncKHR(qemu_egl_display, dmabuf->sync);
> +dmabuf->sync = NULL;
> +}
> +}
> +void egl_dmabuf_wait_sync(QemuDmaBuf *dmabuf)
> +{
Hmm, still the blocking wait. Can't you do something like
"qemu_set_fd_handler(dmabuf->fence_fd, ...)" to avoid the
eglClientWaitSyncKHR() completely?
take care,
Gerd
Re: [PATCH 1/2] hw/misc: Add Infineon DPS310 sensor model
On 6/9/21 12:58 PM, Joel Stanley wrote:
> This contains some hardcoded register values that were obtained from the
> hardware after reading the temperature.
>
> It does enough to test the Linux kernel driver.
It should say briefly what are the device features, which are modeled and
which are not (FIFO, IRQ, sampling configuration).
>
> Signed-off-by: Joel Stanley
> ---
> hw/misc/dps310.c| 357
> hw/arm/Kconfig | 1 +
> hw/misc/Kconfig | 4 +
> hw/misc/meson.build | 1 +
> 4 files changed, 363 insertions(+)
> create mode 100644 hw/misc/dps310.c
>
> diff --git a/hw/misc/dps310.c b/hw/misc/dps310.c
> new file mode 100644
> index ..1fe1a1a645f2
> --- /dev/null
> +++ b/hw/misc/dps310.c
> @@ -0,0 +1,357 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2017 Joel Stanley , IBM Corporation
2021 I guess.
> + *
> + * Infineon DPS310 temperature and himidity sensor
> + *
> + *
> https://www.infineon.com/cms/en/product/sensor/pressure-sensors/pressure-sensors-for-iot/dps310/
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "hw/hw.h"
> +#include "hw/i2c/i2c.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "migration/vmstate.h"
> +
> +#define NUM_REGISTERS 0x32
> +
> +typedef struct DPS310State {
> +/*< private >*/
> +I2CSlave i2c;
> +
> +/*< public >*/
> +uint8_t regs[NUM_REGISTERS];
> +
> +int16_t pressure, temperature;
This is adding state. why not use the DPS310_PRS_B[210] registers
and the DPS310_TMP_B[210] ?
> +
> +uint8_t len;
> +uint8_t buf[2];
> +uint8_t pointer;
> +
> +} DPS310State;
> +
> +typedef struct DPS310Class {
> +I2CSlaveClass parent_class;
> +} DPS310Class;
This definition is unused.
> +
> +#define TYPE_DPS310 "dps310"
> +#define DPS310(obj) OBJECT_CHECK(DPS310State, (obj), TYPE_DPS310)
> +
> +#define DPS310_CLASS(klass) \
> + OBJECT_CLASS_CHECK(DPS310Class, (klass), TYPE_DPS310)
> +#define DPS310_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(DPS310Class, (obj), TYPE_DPS310)
> +
I think we can use OBJECT_DECLARE_SIMPLE_TYPE() instead.
> +#define DPS310_PRS_B2 0x00
> +#define DPS310_PRS_B1 0x01
> +#define DPS310_PRS_B0 0x02
> +#define DPS310_TMP_B2 0x03
> +#define DPS310_TMP_B1 0x04
> +#define DPS310_TMP_B0 0x05
> +#define DPS310_PRS_CFG 0x06
> +#define DPS310_TMP_CFG 0x07
> +#define DPS310_TMP_RATE_BITS GENMASK(6, 4)
GENMASK is a Linux macro.
> +#define DPS310_MEAS_CFG 0x08
> +#define DPS310_MEAS_CTRL_BITS GENMASK(2, 0)
> +#define DPS310_PRESSURE_ENBIT(0)
> +#define DPS310_TEMP_ENBIT(1)
> +#define DPS310_BACKGROUND BIT(2)
> +#define DPS310_PRS_RDY BIT(4)
> +#define DPS310_TMP_RDY BIT(5)
> +#define DPS310_SENSOR_RDY BIT(6)
> +#define DPS310_COEF_RDYBIT(7)
> +#define DPS310_CFG_REG 0x09
> +#define DPS310_RESET0x0c
> +#define DPS310_RESET_MAGIC (BIT(0) | BIT(3))
> +#define DPS310_COEF_BASE0x10
> +#define DPS310_COEF_LAST0x21
> +#define DPS310_COEF_SRC 0x28
> +
> +static void dps310_reset(DeviceState *dev)
> +{
> +DPS310State *s = DPS310(dev);
> +
> +memset(s->regs, 0, sizeof(s->regs));
> +s->pointer = 0;
> +
> +s->regs[0x00] = 0xfe;
> +s->regs[0x01] = 0x2f;
> +s->regs[0x02] = 0xee;
> +s->regs[0x03] = 0x02;
> +s->regs[0x04] = 0x69;
> +s->regs[0x05] = 0xa6;
> +s->regs[0x06] = 0x00;
> +s->regs[0x07] = 0x80;
> +s->regs[0x08] = 0xc7;
> +
> +s->regs[0x0d] = 0x10;
> +s->regs[0x0e] = 0x00;
> +s->regs[0x0f] = 0x00;
> +s->regs[0x10] = 0x0e;
> +s->regs[0x11] = 0x1e;
> +s->regs[0x12] = 0xdd;
> +s->regs[0x13] = 0x13;
> +s->regs[0x14] = 0xca;
> +s->regs[0x15] = 0x5f;
> +s->regs[0x16] = 0x21;
> +s->regs[0x17] = 0x52;
> +s->regs[0x18] = 0xf9;
> +s->regs[0x19] = 0xc6;
> +s->regs[0x1a] = 0x04;
> +s->regs[0x1b] = 0xd1;
> +s->regs[0x1c] = 0xdb;
> +s->regs[0x1d] = 0x47;
> +s->regs[0x1e] = 0x00;
> +s->regs[0x1f] = 0x5b;
> +s->regs[0x20] = 0xfb;
> +s->regs[0x21] = 0x3a;
> +s->regs[0x22] = 0x00;
> +s->regs[0x23] = 0x00;
> +s->regs[0x24] = 0x20;
> +s->regs[0x25] = 0x49;
> +s->regs[0x26] = 0x4e;
> +s->regs[0x27] = 0xa5;
> +s->regs[0x28] = 0x90;
> +
> +s->regs[0x30] = 0x60;
> +s->regs[0x31] = 0x15;
I would put these constant values in a static const array and memset
s->regs with it.
> +
> +/* TODO: assert these after some timeout ? */
> +s->regs[DPS310_MEAS_CFG] = DPS310_COEF_RDY | DPS310_SENSOR_RDY
> +| DPS310_TMP_RDY | DPS310_PRS_RDY;
I think this is fine. Device initialization under QEMU is instantaneous
and sensor values are always ready to be read.
> +}
> +
> +static void dps310_get_pressure(Object *obj, Visitor *v, const char *name,
> +
Re: [PATCH 0/4] modules: add support for target-specific modules.
On Mon, Jun 14, 2021 at 10:19:27PM +, José Ricardo Ziviani wrote: > Hello Gerd, > > On sexta-feira, 11 de junho de 2021 10:03:21 -03 Gerd Hoffmann wrote: > > Hi, > > > > > Are there any pending patches to handle the remaining tcg dependencies > > > in qemu? When trying to build tcg modular (more than only > > > tcg-accel-ops*) I get lots of unresolved symbols to tcg bits which are > > > referenced directly (in cpu.c, gdbstub.c, monitor, ...). > > > > > > The CONFIG_TCG=n case is handled either with stubs or with #ifdef > > > CONFIG_TCG, which doesn't fly for modular tcg ... > > > > So, enough for today, to be continued next week. > > Work branch pushed to > > https://git.kraxel.org/cgit/qemu/log/?h=sirius/modinfo-playground > > > > Topmost patch doesn't compile but shows the build changes. > > I cloned your 'sirius/modinfo-playground-good' and started playing with the > command line options to build these modules. I would like to suggest to > change > the current "--enable-X" with "--X=[enabled,disabled,module]", that seems to > make more sense for these modules. For instance: Hmm, what would be the use case? Right now qemu has the all-or-nothing approach for modules, i.e. if modules are enabled everything we can build as module will be built as module, and I havn't seen any drawbacks so far. So, why would one compile parts of qemu as module and other parts not? Also: when changing this I think it would be good to maintain backward compatibility and use something like this: --enable-tcg=builtin --enable-tcg=module --enable-tcg (use default, probably "module" when modules are enabled and "builtin" otherwise) --disable-tcg take care, Gerd
Re: [PATCH v2 03/18] modules: add qemu-modinfo utility
> > Problem with that approach is that it doesn't work for module > > dependencies ... > > > > Comments on the idea? Suggestions for the module dependency problem? > > Could maybe libbfd be used to find module (symbol) dependencies > > automatically without writing a full dynamic linker? > > Is there any value in exploring use of libclang ? It gives us a real > C parser that we can use to extract information from the C source. In > libvirt we have experimental patches (not yet merged) using libclang to > auto-generate XML parser helpers from struct annotations. It is quite > nice compared to any other hacks for extracting information from C > source files without using a proper parser. libclang can be accessed > from Python3 via its bindings and IIUC should be usable on all our > build platforms Could you do something along the lines of ... (1) find constructors (2) find type_register() calls in the constructor and the TypeInfo structs passed to those calls. (3) inspect the TypeInfo structs to figure the QOM type names. ... with libclang? take care, Gerd
[PATCH v3 2/2] hw/nvme: documentation fix
In the documentation of the '-detached' param "be" and "not" has been used side by side, fix that. Signed-off-by: Gollu Appalanaidu --- hw/nvme/ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index d513b022c4..21883e4b3c 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -114,7 +114,7 @@ * This parameter is only valid together with the `subsys` parameter. If left * at the default value (`false/off`), the namespace will be attached to all * controllers in the NVMe subsystem at boot-up. If set to `true/on`, the - * namespace will be be available in the subsystem not not attached to any + * namespace will be available in the subsystem but not attached to any * controllers. * * Setting `zoned` to true selects Zoned Command Set at the namespace. -- 2.17.1
Re: [PATCH v2 03/18] modules: add qemu-modinfo utility
Hi,
> Another possibility to eschew .o parsing is to add something like this to
> the sources
>
> #ifdef QEMU_MODINFO
> #define MODULE_METADATA(key, value) \
>=<>= MODINFO key value
> #else
> #define MODULE_METADATA(key, value)
> #endif
>
> MODULE_METADATA("opts", "spice")
>
> A Python script could parse compile_commands.json, add -E -DQEMU_MODINFO to
> the command-line option, and look in the output for the metadata.
Hmm, worth trying, although I guess it would be easier to code this up
straight in meson.build and pull the information you need out of the
source set, especially as you'll know then which source files are
compiled into which module.
take care,
Gerd
[PATCH v3 1/2] hw/nvme: fix endianess conversion and add controller list
Add the controller identifiers list CNS 0x13, available list of ctrls
in NVM Subsystem that may or may not be attached to namespaces.
In Identify Ctrl List of the CNS 0x12 and 0x13 no endian conversion
for the nsid field.
These two CNS values shows affect when there exists a Subsystem.
Added condition if there is no Subsystem return invalid field in
command.
Signed-off-by: Gollu Appalanaidu
-v3:
Added condition if there is no Subsystem return invalid field in
command
-v2:
Fix the review comments from Klaus and squashed 2nd commit into
1st commit
---
hw/nvme/ctrl.c | 26 ++
hw/nvme/trace-events | 2 +-
include/block/nvme.h | 1 +
3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 2e7498a73e..d513b022c4 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -4251,9 +4251,11 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n,
NvmeRequest *req, bool active)
return NVME_INVALID_CMD_SET | NVME_DNR;
}
-static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req)
+static uint16_t nvme_identify_ctrl_list(NvmeCtrl *n, NvmeRequest *req,
+bool attached)
{
NvmeIdentify *c = (NvmeIdentify *)>cmd;
+uint32_t nsid = le32_to_cpu(c->nsid);
uint16_t min_id = le16_to_cpu(c->ctrlid);
uint16_t list[NVME_CONTROLLER_LIST_SIZE] = {};
uint16_t *ids = [1];
@@ -4261,15 +4263,21 @@ static uint16_t nvme_identify_ns_attached_list(NvmeCtrl
*n, NvmeRequest *req)
NvmeCtrl *ctrl;
int cntlid, nr_ids = 0;
-trace_pci_nvme_identify_ns_attached_list(min_id);
+trace_pci_nvme_identify_ctrl_list(c->cns, min_id);
-if (c->nsid == NVME_NSID_BROADCAST) {
+if (!n->subsys) {
return NVME_INVALID_FIELD | NVME_DNR;
}
-ns = nvme_subsys_ns(n->subsys, c->nsid);
-if (!ns) {
-return NVME_INVALID_FIELD | NVME_DNR;
+if (attached) {
+if (nsid == NVME_NSID_BROADCAST) {
+return NVME_INVALID_FIELD | NVME_DNR;
+}
+
+ns = nvme_subsys_ns(n->subsys, nsid);
+if (!ns) {
+return NVME_INVALID_FIELD | NVME_DNR;
+}
}
for (cntlid = min_id; cntlid < ARRAY_SIZE(n->subsys->ctrls); cntlid++) {
@@ -4278,7 +4286,7 @@ static uint16_t nvme_identify_ns_attached_list(NvmeCtrl
*n, NvmeRequest *req)
continue;
}
-if (!nvme_ns(ctrl, c->nsid)) {
+if (attached && !nvme_ns(ctrl, nsid)) {
continue;
}
@@ -4493,7 +4501,9 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeRequest
*req)
case NVME_ID_CNS_NS_PRESENT:
return nvme_identify_ns(n, req, false);
case NVME_ID_CNS_NS_ATTACHED_CTRL_LIST:
-return nvme_identify_ns_attached_list(n, req);
+return nvme_identify_ctrl_list(n, req, true);
+case NVME_ID_CNS_CTRL_LIST:
+return nvme_identify_ctrl_list(n, req, false);
case NVME_ID_CNS_CS_NS:
return nvme_identify_ns_csi(n, req, true);
case NVME_ID_CNS_CS_NS_PRESENT:
diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
index ea33d0ccc3..4976eb9bec 100644
--- a/hw/nvme/trace-events
+++ b/hw/nvme/trace-events
@@ -55,7 +55,7 @@ pci_nvme_identify(uint16_t cid, uint8_t cns, uint16_t ctrlid,
uint8_t csi) "cid
pci_nvme_identify_ctrl(void) "identify controller"
pci_nvme_identify_ctrl_csi(uint8_t csi) "identify controller, csi=0x%"PRIx8""
pci_nvme_identify_ns(uint32_t ns) "nsid %"PRIu32""
-pci_nvme_identify_ns_attached_list(uint16_t cntid) "cntid=%"PRIu16""
+pci_nvme_identify_ctrl_list(uint8_t cns, uint16_t cntid) "cns 0x%"PRIx8" cntid
%"PRIu16""
pci_nvme_identify_ns_csi(uint32_t ns, uint8_t csi) "nsid=%"PRIu32",
csi=0x%"PRIx8""
pci_nvme_identify_nslist(uint32_t ns) "nsid %"PRIu32""
pci_nvme_identify_nslist_csi(uint16_t ns, uint8_t csi) "nsid=%"PRIu16",
csi=0x%"PRIx8""
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 0ff9ce17a9..188ab460df 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -980,6 +980,7 @@ enum NvmeIdCns {
NVME_ID_CNS_NS_PRESENT_LIST = 0x10,
NVME_ID_CNS_NS_PRESENT= 0x11,
NVME_ID_CNS_NS_ATTACHED_CTRL_LIST = 0x12,
+NVME_ID_CNS_CTRL_LIST = 0x13,
NVME_ID_CNS_CS_NS_PRESENT_LIST= 0x1a,
NVME_ID_CNS_CS_NS_PRESENT = 0x1b,
NVME_ID_CNS_IO_COMMAND_SET= 0x1c,
--
2.17.1
Re: [PATCH] virtio: disable ioeventfd for record/replay
ping
On 17.05.2021 16:06, Pavel Dovgalyuk wrote:
virtio devices support separate iothreads waiting for
events from file descriptors. These are asynchronous
events that can't be recorded and replayed, therefore
this patch disables ioeventfd for all devices when
record or replay is enabled.
Signed-off-by: Pavel Dovgalyuk
---
hw/s390x/virtio-ccw.c |6 ++
hw/virtio/virtio-mmio.c |6 ++
hw/virtio/virtio-pci.c |6 ++
3 files changed, 18 insertions(+)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 92b950e09a..bd8b9c5755 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -31,6 +31,7 @@
#include "trace.h"
#include "hw/s390x/css-bridge.h"
#include "hw/s390x/s390-virtio-ccw.h"
+#include "sysemu/replay.h"
#define NR_CLASSIC_INDICATOR_BITS 64
@@ -769,6 +770,11 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
+/* fd-based ioevents can't be synchronized in record/replay */
+if (replay_mode != REPLAY_MODE_NONE) {
+dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
+}
+
if (k->realize) {
k->realize(dev, );
if (err) {
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 5952471b38..1af48a1b04 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -29,6 +29,7 @@
#include "qemu/host-utils.h"
#include "qemu/module.h"
#include "sysemu/kvm.h"
+#include "sysemu/replay.h"
#include "hw/virtio/virtio-mmio.h"
#include "qemu/error-report.h"
#include "qemu/log.h"
@@ -740,6 +741,11 @@ static void virtio_mmio_realizefn(DeviceState *d, Error
**errp)
proxy->flags &= ~VIRTIO_IOMMIO_FLAG_USE_IOEVENTFD;
}
+/* fd-based ioevents can't be synchronized in record/replay */
+if (replay_mode != REPLAY_MODE_NONE) {
+proxy->flags &= ~VIRTIO_IOMMIO_FLAG_USE_IOEVENTFD;
+}
+
if (proxy->legacy) {
memory_region_init_io(>iomem, OBJECT(d),
_legacy_mem_ops, proxy,
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b321604d9b..f1e105fa52 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -37,6 +37,7 @@
#include "qemu/range.h"
#include "hw/virtio/virtio-bus.h"
#include "qapi/visitor.h"
+#include "sysemu/replay.h"
#define VIRTIO_PCI_REGION_SIZE(dev) VIRTIO_PCI_CONFIG_OFF(msix_present(dev))
@@ -1760,6 +1761,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
}
+/* fd-based ioevents can't be synchronized in record/replay */
+if (replay_mode != REPLAY_MODE_NONE) {
+proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
+}
+
/*
* virtio pci bar layout used by default.
* subclasses can re-arrange things if needed.
[PATCH] target/ppc/spapr: Update H_GET_CPU_CHARACTERISTICS L1D cache flush bits
There are several new L1D cache flush bits added to the hcall which reflect
hardware security features for speculative cache access issues.
These behaviours are now being specified as negative in order to simplify
patched kernel compatibility with older firmware (a new problem found in
existing systems would automatically be vulnerable).
Signed-off-by: Nicholas Piggin
---
hw/ppc/spapr_hcall.c | 2 ++
include/hw/ppc/spapr.h | 3 +++
2 files changed, 5 insertions(+)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f25014afda..dfd9df469d 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1299,6 +1299,8 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU
*cpu,
behaviour |= H_CPU_BEHAV_L1D_FLUSH_PR;
break;
case SPAPR_CAP_FIXED:
+behaviour |= H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY;
+behaviour |= H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS;
break;
default: /* broken */
assert(safe_cache == SPAPR_CAP_BROKEN);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index f05219f75e..0f25d081a8 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -398,10 +398,13 @@ struct SpaprMachineState {
#define H_CPU_CHAR_THR_RECONF_TRIG PPC_BIT(6)
#define H_CPU_CHAR_CACHE_COUNT_DIS PPC_BIT(7)
#define H_CPU_CHAR_BCCTR_FLUSH_ASSIST PPC_BIT(9)
+
#define H_CPU_BEHAV_FAVOUR_SECURITY PPC_BIT(0)
#define H_CPU_BEHAV_L1D_FLUSH_PRPPC_BIT(1)
#define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR PPC_BIT(2)
#define H_CPU_BEHAV_FLUSH_COUNT_CACHE PPC_BIT(5)
+#define H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY PPC_BIT(7)
+#define H_CPU_BEHAV_NO_L1D_FLUSH_UACCESSPPC_BIT(8)
/* Each control block has to be on a 4K boundary */
#define H_CB_ALIGNMENT 4096
--
2.23.0
Re: [PATCH] target/ppc/spapr: Update H_GET_CPU_CHARACTERISTICS bits
Excerpts from David Gibson's message of May 5, 2021 2:20 pm:
> On Tue, May 04, 2021 at 06:50:54PM +1000, Nicholas Piggin wrote:
>> Excerpts from David Gibson's message of May 4, 2021 10:41 am:
>> > On Mon, May 03, 2021 at 10:58:33PM +1000, Nicholas Piggin wrote:
>> >> There are several new bits added to the hcall which reflect new issues
>> >> found and new hardware mitigations.
>> >>
>> >> This adds the link stack flush behaviour, link stack flush accelerated
>> >> instruction capability, and several L1D flush type behaviours (which are
>> >> now being specified as negative in order to simplify patched kernel
>> >> compatibility with older firmware).
>> >
>> > So, to clarify here, the bits your adding aren't advertising any new
>> > behaviour on qemu/KVM's part, they're just new ways of advertising the
>> > same behaviour?
>>
>> I... think so. "Behaviour" is in context of the hcall that advertises
>> how the processor behaves (or what the guest must do for security).
>
> Heh. Don't get me started on how the difference between
> "characteristics" and "behaviours" in the fields is totally
> non-obvious.
>
>> The new NO_ bits added are for processors that don't require a particular
>> flush. The FLUSH_LINK_STACK was basically always required but I think
>> Linux just keyed off the count cache flush and did both at once.
>>
>> The new LINK_FLUSH_ASSIST is a new processor feature the guest will use
>> to implement link stack flushing, so maybe that does need a cap?
>
> Yeah, sounds like it will.
>
>>
>> >
>> >>
>> >> Signed-off-by: Nicholas Piggin
>> >> ---
>> >> hw/ppc/spapr_hcall.c | 5 +
>> >> include/hw/ppc/spapr.h | 6 ++
>> >> 2 files changed, 11 insertions(+)
>> >>
>> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> >> index 7275d0bba1..f656620232 100644
>> >> --- a/hw/ppc/spapr_hcall.c
>> >> +++ b/hw/ppc/spapr_hcall.c
>> >> @@ -1878,6 +1878,9 @@ static target_ulong
>> >> h_get_cpu_characteristics(PowerPCCPU *cpu,
>> >> behaviour |= H_CPU_BEHAV_L1D_FLUSH_PR;
>> >> break;
>> >> case SPAPR_CAP_FIXED:
>> >> +behaviour |= H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY;
>> >> +behaviour |= H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS;
>> >> +behaviour |= H_CPU_BEHAV_NO_STF_BARRIER;
>> >> break;
>> >> default: /* broken */
>> >> assert(safe_cache == SPAPR_CAP_BROKEN);
>> >> @@ -1909,9 +1912,11 @@ static target_ulong
>> >> h_get_cpu_characteristics(PowerPCCPU *cpu,
>> >> break;
>> >> case SPAPR_CAP_WORKAROUND:
>> >> behaviour |= H_CPU_BEHAV_FLUSH_COUNT_CACHE;
>> >> +behaviour |= H_CPU_BEHAV_FLUSH_LINK_STACK;
>> >> if (count_cache_flush_assist) {
>> >> characteristics |= H_CPU_CHAR_BCCTR_FLUSH_ASSIST;
>> >> }
>> >> +/* Should have a way to enable BCCTR_LINK_FLUSH_ASSIST */
>> >
>> > Do we need a new spapr capability for this link flush thing?
>>
>> It is independent of the FLUSH_COUNT_CACHE capability, so it seems like
>> it should I think? Should that be added as a following patch?
>
> No, it will have to go in first or at the same time. Otherwise we'll
> be errneously advertising things.
Sorry to take so long to get back to this.
I think it might be more complex than I first had. IBS is for indirect
branches except blr type AFAIK, so link stack may still need to be
flushed if count cache is disabled, for example. So then we'd need a
RBS (return branch speculation) or something for the link stack, as well
as the cap for link stack flush assist instruction.
While STF barrier may not be quite right to key off safe cache cap,
maybe it needs its own cap as well? Although CPUs today follow the rule
that if the cache flush is required then so is the stf barrier.
At any rate, the cache flush ones are most important so I'll send a
patch for them alone first.
Thanks,
Nick
Re: [PATCH 2/2] arm/aspeed: Add DPS310 to Witherspoon and Rainier
On 6/9/21 12:58 PM, Joel Stanley wrote: > Witherspoon uses the DPS310 as a temperature sensor. Rainier uses it as > a temperature and humidity sensor. > > Signed-off-by: Joel Stanley Reviewed-by: Cédric Le Goater Thanks, C. > --- > hw/arm/aspeed.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 0eafc791540d..619ad869dd71 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -597,7 +597,6 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState > *bmc) > > /* Bus 3: TODO bmp280@77 */ > /* Bus 3: TODO max31785@52 */ > -/* Bus 3: TODO dps310@76 */ > dev = DEVICE(i2c_slave_new(TYPE_PCA9552, 0x60)); > qdev_prop_set_string(dev, "description", "pca1"); > i2c_slave_realize_and_unref(I2C_SLAVE(dev), > @@ -612,6 +611,7 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState > *bmc) > qdev_connect_gpio_out(dev, pca1_leds[i].gpio_id, >qdev_get_gpio_in(DEVICE(led), 0)); > } > +i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 3), "dps310", > 0x76); > i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 4), "tmp423", > 0x4c); > i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 5), "tmp423", > 0x4c); > > @@ -693,9 +693,9 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc) > i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 6), TYPE_TMP105, > 0x4b); > > -/* Bus 7: TODO dps310@76 */ > /* Bus 7: TODO max31785@52 */ > i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 7), "pca9552", > 0x61); > +i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 7), "dps310", > 0x76); > /* Bus 7: TODO si7021-a20@20 */ > i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 7), TYPE_TMP105, > 0x48); >
Re: [PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it
> On Jun 14, 2021, at 9:20 PM, Richard Henderson > wrote: > > On 6/13/21 2:15 PM, Jason Thorpe wrote: >> +/* The following was copied from hw/isa/i82378.c and modified to provide >> + only the minimal PCI device node. */ >> + >> +/* >> + * QEMU Intel i82378 emulation (PCI to ISA bridge) >> + * > > Why can't we just use the existing device model? > Certainly duplicating code like this isn't the best way. Yah, I’m not super happy with that, either, tbh. When I first started working on this several months ago, I it looked like it would be invasive to wire it up in the way the Alpha platform expects, but I can’t remember exactly what the issue was. Anyways, I’ll look at it again. Stay tuned. -- thorpej
Re: [PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it
On 6/13/21 2:15 PM, Jason Thorpe wrote: +/* The following was copied from hw/isa/i82378.c and modified to provide + only the minimal PCI device node. */ + +/* + * QEMU Intel i82378 emulation (PCI to ISA bridge) + * Why can't we just use the existing device model? Certainly duplicating code like this isn't the best way. r~
Re: [PATCH 1/4] mc146818rtc: Make PF independent of PIE
Cc: paolo and mst
On 6/13/21 2:15 PM, Jason Thorpe wrote:
Make the PF flag behave like real hardware by always running the
periodic timer without regard to the setting of the PIE bit, so
that the PF will be set when the period expires even if an interrupt
will not be raised. This behavior is documented on page 16 of the
MC146818A advance information datasheet.
Signed-off-by: Jason Thorpe
---
hw/rtc/mc146818rtc.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 4fbafddb22..366b8f13de 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
{
int period_code;
-if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
-return 0;
- }
This looks correct, but I don't know enough about this device.
r~
Re: [PATCH 4/4] alpha: Provide console information to the PALcode at start-up.
On 6/13/21 2:15 PM, Jason Thorpe wrote: Redefine the a2 register passed by Qemu at start-up to also include some configuration flags, in addition to the CPU count, and define a flag to mirror the "-nographics" option. Signed-off-by: Jason Thorpe --- hw/alpha/dp264.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c index ac97104464..d86dcb1d81 100644 --- a/hw/alpha/dp264.c +++ b/hw/alpha/dp264.c @@ -72,9 +72,20 @@ static void clipper_init(MachineState *machine) cpus[i] = ALPHA_CPU(cpu_create(machine->cpu_type)); } +/* arg0 -> memory size + arg1 -> kernel entry point + arg2 -> config word + + Config word: bits 0-5 -> ncpus +bit 6 -> nographics option (for HWRPB CTB) + + See init_hwrpb() in the PALcode. */ + cpus[0]->env.trap_arg0 = ram_size; cpus[0]->env.trap_arg1 = 0; cpus[0]->env.trap_arg2 = smp_cpus; +if (!machine->enable_graphics) + cpus[0]->env.trap_arg2 |= (1 << 6); Thanks, I have fixed up the style problems and queued. r~
Re: [PATCH 2/4] alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.
On 6/13/21 2:15 PM, Jason Thorpe wrote: Since we are emulating a Clipper device topology, we need to set the minimum PCI device ID to 1, as there is no IRQ mapping for a device at ID 0 (see sys_dp264.c:clipper_map_irq()). - Add a 'devfn_min' argument to typhoon_init(). Pass that argument along to pci_register_root_bus(). - In clipper_init(), pass PCI_DEVFN(1, 0) as the minimum PCI device ID/function. Signed-off-by: Jason Thorpe --- hw/alpha/alpha_sys.h | 2 +- hw/alpha/dp264.c | 5 +++-- hw/alpha/typhoon.c | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) Thanks, queued. r~
Re: [RFC] GitLab issue tracker labeling process: arch/target, os, and accel labels
On Mon, Jun 14, 2021 at 01:32:11PM -0400, John Snow wrote:
> Hi, I'd like to work out our collective preferences for how we triage issues
> that concern the execution environment; namely the arch (now "target", os,
> and accel labels.
>
> If you're CC'd on this mail, you're either listed as a TCG maintainer, a
> target maintainer, or have been heavily involved in the GitLab issue tracker
> migration process. You might care about how we triage and file these issues.
>
> I'm sending the mail because I've been (so far) responsible for a lot of the
> labeling as we move over from Launchpad. I'll need to change my process to
> accommodate what our maintainers want to see. (And to encourage others to
> join in on the triage process.)
>
> As of right now, there's no formal process or document for how we use any of
> the labels -- Thomas and I had been only informally collaborating in our
> usage of them as we migrated from Launchpad. Thomas has a script he uses to
> move the bugs, so I usually don't edit these labels until I give him a heads
> up so I don't break his script.
>
> Let's discuss what changes we need to make and collaborate on when and how
> we'll make them.
In general, what's the convention when a bug is independent of (say)
the accel: does it get none of the accel tags, or all of them?
Likewise with OS and the other categories.
> # Accel
>
> Currently "accel: XXX", for HAX, HVF, KVM, TCG, WHPX and Xen.
>
> https://gitlab.com/qemu-project/qemu/-/labels?subscribed==accel%3A
>
> Multiple accel labels can be applied to an issue, not just one. The intent
> was to allow for issues that may impact multiple accelerators, though that
> case may be rare.
>
> I think these are reasonably straightforward and unambiguous, though for
> some qemu-system reports it's not always evident which accelerator applies
> right away without more info. The accel tag is often omitted because of this
> uncertainty.
>
> I'd like to keep the mapping here 1:1 against ACCEL_CLASS_NAME if I can. It
> makes the mapping from CLI to accel tag fairly straightforward.
>
> We technically lack a "qtest" accel tag for that parity. It could be added,
> but it's likely not actually useful versus the "tests" label. It's not
> really a user-facing accelerator.
>
> I see we also have a new "nvmm" accelerator, too, which should now be added
> here.
>
> RTH raises the issue of the "TCI" subsystem of TCG, which is not a full
> accelerator in its own right, but (I think) a special case of TCG. If I keep
> the 1:1 mapping to ACCEL_CLASS_NAME, "accel: TCI" is inappropriate.
>
> Some suggestions:
> - "TCI" by itself, simple enough.
> - "TCG-TCI" or "TCG: TCI" or "TCG/TCI" or similar, so that it shows up in
> label search when you search for 'tcg'.
> - "accel: TCG:TCI". Similar to above but uses the "accel:" prefix too.
>
> My only concern here is completeness of the label: this one seems like it's
> at particular risk of being forgotten or lost. It works perfectly well as an
> organizational bucket for people working on TCI, but I wonder if it will
> work well as an "issue inbox". Intended use begins to matter here. Your
> thoughts, Stefan?
>
>
> # OS
>
> Currently "os: XXX" for BSD, Linux, Windows, and macOS.
>
> https://gitlab.com/qemu-project/qemu/-/labels?subscribed==os%3A
>
> Multiple OS labels can be applied to an issue.
>
> Originally, we kept this label somewhat vague and have been using it to
> identify both the host AND guest involved with an issue.
>
> Stefan Weil has requested that we refactor this to separate the concerns so
> that he can identify issues where Windows is the host without wading through
> numerous reports where Windows is merely the guest. Reasonable request.
>
> Shall we split it into "host: XXX" and "guest: XXX" for {BSD, Linux,
> Windows, macOS}?
I think that would be a good idea. Except maybe "host-os" and
"guest-os", because "host" in particular is ambiguous with host
architecture. (not relevant that often, but sometimes).
> This isn't too hard to do at initial triage time, but we'll need to sift
> through the bugs we've labeled so far and re-label them. Help on this would
> be appreciated. I would prefer we create a *new* set of labels and then draw
> down on the old labels instead of just renaming them. That way, the old
> label can be used as a re-triage queue.
That seems like the right way to do it to me.
> # arch/target
>
> Currently "target: XXX" for alpha, arm, avr, cris, hexagon, hppa, i386,
> m68k, microblaze, mips, nios2, openrisc, ppc, riscv, rx, s390x, sh4, sparc,
> tricore, xtensa.
>
> https://gitlab.com/qemu-project/qemu/-/labels?subscribed==target%3A
>
> The names map 1:1 to the directories in target/.
> The names in [square brackets] in the label descriptions correspond 1:1 with
> the SysEmuTarget QAPI enum defined in qapi/machine.json.
>
> Multiple target labels can be applied to an issue. Originally, this was
> named "arch", so this was to allow multiple
Re: [PATCH v2 0/1] PALcode fixes for guest OS console initialization
On 6/14/21 5:29 PM, Jason Thorpe wrote: On Jun 13, 2021, at 2:09 PM, Jason Thorpe wrote: but there is still some research going on about how real DEC SRM initializes a particular field in various circumstances; this is my best guess based on available documentation and observed behavior of real machines, and is sufficient for the BSD operating systems. FWIW, I have since confirmed my hypothesis about how genuine DEC SRM fills in the "type" and "term_type" fields on 2 different generations of PCI-based Alpha systems (i.e. it matches the patch submitted here). Excellent, thanks. Queued. r~
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus
On 6/14/21 6:41 PM, David Gibson wrote: I think move these to mmu-book3s-v3.h, since they're correct for both the radix and hash sides of the modern book3s mmu. They're also correct for all non-booke mmus, e.g. hash32 and 6xx, which is why I recommended internal.h (or some new mmu-internal.h). While neither hash32 nor 6xx have HV, and thus there is no second tlb bug, it would still be More Correct to use mmu_idx instead of direct references to msr_pr et al. r~
[PATCH 3/5] target/s390x: Improve s390_cpu_dump_state vs cc_op
Use s390_cpu_get_psw_mask so that we print the correct
architectural value of psw.mask. Do not print cc_op
unless tcg_enabled.
Signed-off-by: Richard Henderson
---
target/s390x/helper.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 559fc3573f..1445b74451 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -338,12 +338,14 @@ void s390_cpu_dump_state(CPUState *cs, FILE *f, int flags)
CPUS390XState *env = >env;
int i;
-if (env->cc_op > 3) {
-qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc
%15s\n",
- env->psw.mask, env->psw.addr, cc_name(env->cc_op));
+qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64,
+ s390_cpu_get_psw_mask(env), env->psw.addr);
+if (!tcg_enabled()) {
+qemu_fprintf(f, "\n");
+} else if (env->cc_op > 3) {
+qemu_fprintf(f, " cc %15s\n", cc_name(env->cc_op));
} else {
-qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc
%02x\n",
- env->psw.mask, env->psw.addr, env->cc_op);
+qemu_fprintf(f, " cc %02x\n", env->cc_op);
}
for (i = 0; i < 16; i++) {
--
2.25.1
[PATCH 5/5] linux-user/s390x: Save and restore psw.mask properly
At present, we're referencing env->psw.mask directly, which
fails to ensure that env->cc_op is incorporated or updated.
Use s390_cpu_{set_psw,get_psw_mask} to fix this.
Mirror the kernel's cleaning of the psw.mask in save_sigregs
and restore_sigregs. Ignore PSW_MASK_RI for now, as qemu does
not support that.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 37 -
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index ef136dae33..bf8a8fbfe9 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -112,15 +112,23 @@ get_sigframe(struct target_sigaction *ka, CPUS390XState
*env, size_t frame_size)
return (sp - frame_size) & -8ul;
}
+#define PSW_USER_BITS (PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \
+ PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_ASC_PRIMARY)
+#define PSW_MASK_USER (PSW_MASK_ASC | PSW_MASK_CC | PSW_MASK_PM | \
+ PSW_MASK_64 | PSW_MASK_32)
+
static void save_sigregs(CPUS390XState *env, target_sigregs *sregs)
{
+uint64_t psw_mask = s390_cpu_get_psw_mask(env);
int i;
/*
* Copy a 'clean' PSW mask to the user to avoid leaking
* information about whether PER is currently on.
+ * TODO: qemu does not support PSW_MASK_RI; it will never be set.
*/
-__put_user(env->psw.mask, >regs.psw.mask);
+psw_mask = PSW_USER_BITS | (psw_mask & PSW_MASK_USER);
+__put_user(psw_mask, >regs.psw.mask);
__put_user(env->psw.addr, >regs.psw.addr);
for (i = 0; i < 16; i++) {
@@ -289,7 +297,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
{
-target_ulong prev_addr;
+uint64_t prev_addr, prev_mask, mask, addr;
int i;
for (i = 0; i < 16; i++) {
@@ -297,9 +305,28 @@ static void restore_sigregs(CPUS390XState *env,
target_sigregs *sc)
}
prev_addr = env->psw.addr;
-__get_user(env->psw.mask, >regs.psw.mask);
-__get_user(env->psw.addr, >regs.psw.addr);
-trace_user_s390x_restore_sigregs(env, env->psw.addr, prev_addr);
+__get_user(mask, >regs.psw.mask);
+__get_user(addr, >regs.psw.addr);
+trace_user_s390x_restore_sigregs(env, addr, prev_addr);
+
+/*
+ * Use current psw.mask to preserve PER bit.
+ * TODO:
+ * if (!is_ri_task(current) && (user_sregs.regs.psw.mask & PSW_MASK_RI))
+ * return -EINVAL;
+ * Simply do not allow it to be set in mask.
+ */
+prev_mask = s390_cpu_get_psw_mask(env);
+mask = (prev_mask & ~PSW_MASK_USER) | (mask & PSW_MASK_USER);
+/* Check for invalid user address space control. */
+if ((mask & PSW_MASK_ASC) == PSW_ASC_HOME) {
+mask = (mask & ~PSW_MASK_ASC) | PSW_ASC_PRIMARY;
+}
+/* Check for invalid amode. */
+if (mask & PSW_MASK_64) {
+mask |= PSW_MASK_32;
+}
+s390_cpu_set_psw(env, mask, addr);
for (i = 0; i < 16; i++) {
__get_user(env->aregs[i], >regs.acrs[i]);
--
2.25.1
[PATCH 4/5] target/s390x: Use s390_cpu_{set_psw, get_psw_mask} in gdbstub
No change in behaviour, as gdbstub was correctly written to
install and extract the cc value.
Signed-off-by: Richard Henderson
---
target/s390x/gdbstub.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/target/s390x/gdbstub.c b/target/s390x/gdbstub.c
index d6fce5ff1e..5b4e38a13b 100644
--- a/target/s390x/gdbstub.c
+++ b/target/s390x/gdbstub.c
@@ -31,18 +31,10 @@ int s390_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
{
S390CPU *cpu = S390_CPU(cs);
CPUS390XState *env = >env;
-uint64_t val;
-int cc_op;
switch (n) {
case S390_PSWM_REGNUM:
-if (tcg_enabled()) {
-cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
-env->cc_vr);
-val = deposit64(env->psw.mask, 44, 2, cc_op);
-return gdb_get_regl(mem_buf, val);
-}
-return gdb_get_regl(mem_buf, env->psw.mask);
+return gdb_get_regl(mem_buf, s390_cpu_get_psw_mask(env));
case S390_PSWA_REGNUM:
return gdb_get_regl(mem_buf, env->psw.addr);
case S390_R0_REGNUM ... S390_R15_REGNUM:
@@ -59,10 +51,7 @@ int s390_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
switch (n) {
case S390_PSWM_REGNUM:
-env->psw.mask = tmpl;
-if (tcg_enabled()) {
-env->cc_op = extract64(tmpl, 44, 2);
-}
+s390_cpu_set_psw(env, tmpl, env->psw.addr);
break;
case S390_PSWA_REGNUM:
env->psw.addr = tmpl;
--
2.25.1
[PATCH 0/5] linux-user/s390x: Fix psw.mask handling in signals
The PSW_MASK_CC component of psw.mask was not handled properly
in the creation or restoration of signal frames.
r~
Richard Henderson (5):
target/s390x: Expose load_psw and get_psw_mask to cpu.h
target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
target/s390x: Improve s390_cpu_dump_state vs cc_op
target/s390x: Use s390_cpu_{set_psw,get_psw_mask} in gdbstub
linux-user/s390x: Save and restore psw.mask properly
target/s390x/cpu.h | 3 ++
target/s390x/internal.h| 5 --
linux-user/s390x/signal.c | 37 --
target/s390x/cc_helper.c | 2 +-
target/s390x/excp_helper.c | 28 +-
target/s390x/gdbstub.c | 15 +-
target/s390x/helper.c | 101 -
target/s390x/sigp.c| 3 +-
8 files changed, 110 insertions(+), 84 deletions(-)
--
2.25.1
[PATCH 1/5] target/s390x: Expose load_psw and get_psw_mask to cpu.h
Rename to s390_cpu_set_psw and s390_cpu_get_psw_mask at the
same time. Adjust so that they compile for user-only.
Signed-off-by: Richard Henderson
---
target/s390x/cpu.h | 3 ++
target/s390x/internal.h| 5 ---
target/s390x/cc_helper.c | 2 +-
target/s390x/excp_helper.c | 28 ++--
target/s390x/helper.c | 89 --
target/s390x/sigp.c| 3 +-
6 files changed, 69 insertions(+), 61 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 2464d4076c..b26ae8fff2 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -845,6 +845,9 @@ int s390_cpu_pv_mem_rw(S390CPU *cpu, unsigned int offset,
void *hostbuf,
int s390_cpu_restart(S390CPU *cpu);
void s390_init_sigp(void);
+/* helper.c */
+void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
+uint64_t s390_cpu_get_psw_mask(CPUS390XState *env);
/* outside of target/s390x/ */
S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 11515bb617..4a18b74d10 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -235,10 +235,6 @@ int s390_cpu_write_elf64_note(WriteCoreDumpFunction f,
CPUState *cs,
const char *cc_name(enum cc_op cc_op);
uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t
dst,
uint64_t vr);
-#ifndef CONFIG_USER_ONLY
-void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
-#endif /* CONFIG_USER_ONLY */
-
/* cpu.c */
#ifndef CONFIG_USER_ONLY
@@ -303,7 +299,6 @@ void s390_cpu_gdb_init(CPUState *cs);
void s390_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
void do_restart_interrupt(CPUS390XState *env);
#ifndef CONFIG_USER_ONLY
-uint64_t get_psw_mask(CPUS390XState *env);
void s390_cpu_recompute_watchpoints(CPUState *cs);
void s390x_tod_timer(void *opaque);
void s390x_cpu_timer(void *opaque);
diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index e7039d0d18..e7a74d66dd 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -509,7 +509,7 @@ uint32_t HELPER(calc_cc)(CPUS390XState *env, uint32_t
cc_op, uint64_t src,
#ifndef CONFIG_USER_ONLY
void HELPER(load_psw)(CPUS390XState *env, uint64_t mask, uint64_t addr)
{
-load_psw(env, mask, addr);
+s390_cpu_set_psw(env, mask, addr);
cpu_loop_exit(env_cpu(env));
}
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 20625c2c8f..9c361428c8 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -252,7 +252,7 @@ static void do_program_interrupt(CPUS390XState *env)
lowcore->pgm_ilen = cpu_to_be16(ilen);
lowcore->pgm_code = cpu_to_be16(env->int_pgm_code);
-lowcore->program_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+lowcore->program_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->program_old_psw.addr = cpu_to_be64(env->psw.addr);
mask = be64_to_cpu(lowcore->program_new_psw.mask);
addr = be64_to_cpu(lowcore->program_new_psw.addr);
@@ -260,7 +260,7 @@ static void do_program_interrupt(CPUS390XState *env)
cpu_unmap_lowcore(lowcore);
-load_psw(env, mask, addr);
+s390_cpu_set_psw(env, mask, addr);
}
static void do_svc_interrupt(CPUS390XState *env)
@@ -272,14 +272,14 @@ static void do_svc_interrupt(CPUS390XState *env)
lowcore->svc_code = cpu_to_be16(env->int_svc_code);
lowcore->svc_ilen = cpu_to_be16(env->int_svc_ilen);
-lowcore->svc_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+lowcore->svc_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->svc_old_psw.addr = cpu_to_be64(env->psw.addr + env->int_svc_ilen);
mask = be64_to_cpu(lowcore->svc_new_psw.mask);
addr = be64_to_cpu(lowcore->svc_new_psw.addr);
cpu_unmap_lowcore(lowcore);
-load_psw(env, mask, addr);
+s390_cpu_set_psw(env, mask, addr);
/* When a PER event is pending, the PER exception has to happen
immediately after the SERVICE CALL one. */
@@ -348,12 +348,12 @@ static void do_ext_interrupt(CPUS390XState *env)
mask = be64_to_cpu(lowcore->external_new_psw.mask);
addr = be64_to_cpu(lowcore->external_new_psw.addr);
-lowcore->external_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+lowcore->external_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
lowcore->external_old_psw.addr = cpu_to_be64(env->psw.addr);
cpu_unmap_lowcore(lowcore);
-load_psw(env, mask, addr);
+s390_cpu_set_psw(env, mask, addr);
}
static void do_io_interrupt(CPUS390XState *env)
@@ -373,7 +373,7 @@ static void do_io_interrupt(CPUS390XState *env)
lowcore->subchannel_nr = cpu_to_be16(io->nr);
lowcore->io_int_parm = cpu_to_be32(io->parm);
lowcore->io_int_word = cpu_to_be32(io->word);
-lowcore->io_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+lowcore->io_old_psw.mask = cpu_to_be64(s390_cpu_get_psw_mask(env));
[PATCH 2/5] target/s390x: Do not modify cpu state in s390_cpu_get_psw_mask
We want to use this function for debugging, and debug should
not modify cpu state (even non-architectural cpu state) lest
we introduce heisenbugs.
Signed-off-by: Richard Henderson
---
target/s390x/helper.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index d311903b94..559fc3573f 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -321,12 +321,12 @@ uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
uint64_t r = env->psw.mask;
if (tcg_enabled()) {
-env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst,
- env->cc_vr);
+uint64_t cc = calc_cc(env, env->cc_op, env->cc_src,
+ env->cc_dst, env->cc_vr);
+assert(cc <= 3);
r &= ~PSW_MASK_CC;
-assert(!(env->cc_op & ~3));
-r |= (uint64_t)env->cc_op << 44;
+r |= cc << 44;
}
return r;
--
2.25.1
RE: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command for COLO passthrough
> -Original Message-
> From: Markus Armbruster
> Sent: Friday, June 11, 2021 5:37 PM
> To: Zhang, Chen
> Cc: Eric Blake ; Lukas Straub ;
> Daniel P.Berrangé ; Li Zhijian
> ; Jason Wang ; Markus
> Armbruster ; qemu-dev ;
> Gerd Hoffmann ; Zhang Chen ;
> Dr. David Alan Gilbert
> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP command
> for COLO passthrough
>
> "Zhang, Chen" writes:
>
> >> -Original Message-
> >> From: Eric Blake
> >> Sent: Friday, June 4, 2021 10:35 PM
> >> To: Zhang, Chen
> >> Cc: Jason Wang ; qemu-dev >> de...@nongnu.org>; Dr. David Alan Gilbert ;
> >> Markus Armbruster ; Daniel P. Berrangé
> >> ; Gerd Hoffmann ; Li
> Zhijian
> >> ; Zhang Chen ; Lukas
> >> Straub
> >> Subject: Re: [PATCH V7 1/6] qapi/net: Add IPFlowSpec and QMP
> command
> >> for COLO passthrough
> >>
> >> On Wed, May 26, 2021 at 10:54:19AM +0800, Zhang Chen wrote:
> >> > Since the real user scenario does not need COLO to monitor all traffic.
> >> > Add colo-passthrough-add and colo-passthrough-del to maintain a
> >> > COLO network passthrough list. Add IPFlowSpec struct for all QMP
> commands.
> >> > Except protocol field is necessary, other fields are optional.
> >>
> >> That last sentence reads awkwardly, and I don't see a protocol field
> >> in the patch below.
> >
> > Oh, We move the protocol field to optional by Lukas's comments in V6.
> > I will remove this comments here.
> >
> >>
> >> >
> >> > Signed-off-by: Zhang Chen
> >> > ---
> >> > net/net.c | 10
> >> > qapi/net.json | 68
> >> > +++
> >> > 2 files changed, 78 insertions(+)
> >> >
> >>
> >> > +++ b/qapi/net.json
> >> > @@ -7,6 +7,7 @@
> >> > ##
> >> >
> >> > { 'include': 'common.json' }
> >> > +{ 'include': 'sockets.json' }
> >> >
> >> > ##
> >> > # @set_link:
> >> > @@ -694,3 +695,70 @@
> >> > ##
> >> > { 'event': 'FAILOVER_NEGOTIATED',
> >> >'data': {'device-id': 'str'} }
> >> > +
> >> > +##
> >> > +# @IPFlowSpec:
> >> > +#
> >> > +# IP flow specification.
> >> > +#
> >> > +# @protocol: Transport layer protocol like TCP/UDP...
> >>
> >> Why is this open-coded as 'str' instead of an enum?
> >
> > The original code use enum, but we change it by Dave and Markus's
> comments.
> > Please check the history:
> > https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg03919.html
>
> It's a string to be passed to getprotobyname(3). Please mention that in the
> doc string.
OK, I will add this to commit log and qapi/net.json in next version.
>
> It's not an enum, because we don't want to duplicate /etc/protocols in the
> QAPI schema.
Yes.
>
> >> > +#
> >> > +# @object-name: Point out the IPflow spec effective range of
> >> > +object,
>
> I have no idea what that means :)
>
> It might be what was called @id in v4. There, you explained
>
> The @id means packet hander in Qemu. Because not all the guest network
> packet into the colo-compare module, the net-filters are same cases.
> There modules attach to NIC or chardev socket to work, VM maybe have
> multi modules running. So we use the ID to set the rule to the specific
> module.
>
> and I asked you to work it into the doc comment.
>
Yes, As we discussed In V4, already changed the "id" to "object-name".
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01088.html
And sorry I missed another mail thread to add the comments, I will add the
explanation and quick update to V8 for this series.
Thanks for your help~
Thanks
Chen
> If you want help with working it into the doc comment, please explain its
> intended use for dummies :)
>
> >> > +# If there is no such part, it means global spec.
> >> > +#
> >> > +# @source: Source address and port.
> >> > +#
> >> > +# @destination: Destination address and port.
> >> > +#
> >> > +# Since: 6.1
> >> > +##
> >> > +{ 'struct': 'IPFlowSpec',
> >> > + 'data': { '*protocol': 'str', '*object-name': 'str',
> >> > +'*source': 'InetSocketAddressBase',
> >> > +'*destination': 'InetSocketAddressBase' } }
> >> > +
> >> > +##
> >> > +# @colo-passthrough-add:
> >> > +#
> >> > +# Add passthrough entry according to user's needs in COLO-compare.
> >> > +# Source IP/port and destination IP/port both optional, If user
> >> > +just # input parts of infotmation, it will match all.
> >>
> >> information
> >>
> >> Grammar suggestion:
> >>
> >> The source and destination IP/ports are both optional; if the user
> >> only inputs part of the information, this will match all traffic.
> >>
> >> except I'm not sure if my rewrite conveys the actual intent.
> >
> > Looks good to me, It should add the "protocol" to optional too.
> > Sorry, I'm not a native speaker, I will fix it in next version.
> >
> >>
> >> > +#
> >> > +# Returns: Nothing on success
> >> > +#
> >> > +# Since: 6.1
> >> > +#
> >> > +# Example:
> >> > +#
> >> > +# -> { "execute": "colo-passthrough-add",
> >> > +# "arguments": { "protocol": "tcp", "object-name": "object0",
Re: [PATCH v4 6/8] hw/intc: GICv3 redistributor ITS processing
On Jun 11 2021, at 4:30 am, Peter Maydell wrote: > On Fri, 11 Jun 2021 at 00:39, Shashi Mallela > wrote: > > > > Have addressed all comments except the ones with responses(inline) below:- > > > > On Jun 8 2021, at 9:57 am, Peter Maydell wrote: > > > > > + cs->lpivalid = false; > > > + cs->hpplpi.prio = 0xff; > > > + gicv3_redist_update_lpi(cs); > > > > You can avoid doing a full update a lot of the time: > > * if this LPI is worse than the current value in hpplpi > > (where by "worse" I mean lower-priority by the same kind of > > comparison irqbetter() does) then we haven't changed the best-available > > pending LPI, so we don't need to do an update > > * if we set the pending bit to 1 and the LPI is enabled and the priority > > of this LPI is better than the current hpplpi, then we know this LPI > > is now the best, so we can just set hpplpi.prio and .irq without > > doing a full rescan > > * if we didn't actually change the value of the pending bit, we > > don't need to do an update (you get this for free if you take the > > simplification suggestion I make above, which does an early-return > > in the "no change" case) > > > > > Accepted the code simplification,but by not calling > > > gicv3_redist_update_lpi(cs) here ,the scenario of an activated LPI fails > > > because > > this LPI's priority (which could be lower than current hpplpi) is never > > checked/updated and gicv3_redist_update_noirqset() fails to present a valid > > active high priority LPI(if applicable) to the cpu,since it is always > > checking against a stale hpplpi info. > > If the LPI is lower priority (higher number) than the current > hpplpi then it would not change the existing hpplpi info in > a full-scan. If the LPI being activated is higher priority > (lower number) than the current hpplpi then that is my point 2 above, > and we set it as the hpplpi without needing the full-scan. And for > the other cases (eg highest-priority LPI being deactivated) we > should fall through to the default "call update_lpi" case. > > So I don't really understand why this wouldn't work. > -- PMM Have got this working as per comments.Please ignore my last comment.
Re: [PATCH v3 1/2] hw/char: sifive_uart
On Tue, Jun 15, 2021 at 10:19 AM Bin Meng wrote: > > On Sun, Jun 13, 2021 at 10:12 PM Lukas Jünger > wrote: > > > > Make function names consistent > > > > Signed-off-by: Lukas Jünger > > --- > > hw/char/sifive_uart.c | 46 ++- > > 1 file changed, 24 insertions(+), 22 deletions(-) > > > > Reviewed-by: Bin Meng Please reword the commit title to make it something meaningful. Please do the same for patch 2. Regards, Bin
Re: [PATCH v3 1/2] hw/char: sifive_uart
On Sun, Jun 13, 2021 at 10:12 PM Lukas Jünger wrote: > > Make function names consistent > > Signed-off-by: Lukas Jünger > --- > hw/char/sifive_uart.c | 46 ++- > 1 file changed, 24 insertions(+), 22 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH] meson.build: Support ncurses on MacOS
On 6/14/2021 1:45 AM, Philippe Mathieu-Daudé wrote: On 6/13/21 8:33 AM, Stefan Weil wrote: Am 13.06.21 um 03:40 schrieb Brad Smith: This same problem also applies to OpenBSD as we have the same version of ncurses with support for wide characters. I have a similar patch in our QEMU port. Then we should either extend the conditional statement to handle OpenBSD as well, or simply define both macros unconditionally: # Newer versions of curses use NCURSES_WIDECHAR. # Older versions (e. g. on MacOS, OpenBSD) still require _XOPEN_SOURCE_EXTENDED. curses_compile_args = ['-DNCURSES_WIDECHAR=1', '-D_XOPEN_SOURCE_EXTENDED=1'] Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new versions, so that's another option. It is simpler to ask Brad to upstream the OpenBSD patch :) That doesn't answer his question and that's the part that actually matters.
Re: [RFC PATCH v2 2/2] target/ppc: make gdb able to translate priviledged addresses
On Mon, Jun 14, 2021 at 09:37:54PM +0200, Philippe Mathieu-Daudé wrote:
> On 6/14/21 9:16 PM, Bruno Larsen (billionai) wrote:
> > This patch changes ppc_cpu_get_phys_page_debug so that it is now
> > able to translate both, priviledged and real mode addresses
> > independently of whether the CPU executing it has those permissions
> >
> > This was mentioned by Fabiano as something that would be very useful to
> > help with debugging, but could possibly constitute a security issue if
> > that debug function can be called in some way by prodution code. the
> > solution was implemented such that it would be trivial to wrap it around
> > ifdefs for building only with --enable-debug, for instance, but we are
> > not sure this is the best approach, hence why it is an RFC.
> >
> > Suggested-by: Fabiano Rosas
> > Signed-off-by: Bruno Larsen (billionai)
> > ---
> > target/ppc/mmu_helper.c | 23 +++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> > index 9dcdf88597..41c727c690 100644
> > --- a/target/ppc/mmu_helper.c
> > +++ b/target/ppc/mmu_helper.c
> > @@ -2947,6 +2947,29 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs,
> > vaddr addr)
> >cpu_mmu_index(>env, true), false)) {
> > return raddr & TARGET_PAGE_MASK;
> > }
> > +
> > +/*
> > + * This is a fallback, in case we're asking for priviledged memory to
> > + * be printed, but the PCU is not executing in a priviledged
> > manner.
s/PCU/CPU/
> > + *
> > + * The code could be considered a security vulnerability if
> > + * this function can be called in a scenario that does not involve
> > + * debugging.
> > + * Given the name and how useful using real addresses may be for
> > + * actually debugging, however, we decided to include it anyway and
> > + * discuss how to best avoid the possible security concerns.
> > + * The current plan is that, if there is a chance this code is called
> > in
> > + * a production environment, we can surround it with ifdefs so that it
> > + * is only compiled with --enable-debug
>
> Nothing forbid us to use --enable-debug in a production environment.
>
> > + */
> > +/* attempt to translate first with virtual addresses */
> > +if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, , , , 1, false) ||
> > +ppc_xlate(cpu, addr, MMU_INST_FETCH, , , , 1, false) ||
> > +/* if didn't work, attempt to translate with real addresses */
> > +ppc_xlate(cpu, addr, MMU_DATA_LOAD, , , , 3, false) ||
> > +ppc_xlate(cpu, addr, MMU_INST_FETCH, , , , 3, false)) {
> > +return raddr & TARGET_PAGE_MASK;
> > +}
> > return -1;
> > }
> >
> >
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus
On Mon, Jun 14, 2021 at 06:04:13PM -0300, Fabiano Rosas wrote:
> "Bruno Larsen (billionai)" writes:
>
> > +/*
> > + * These correspond to the mmu_idx values computed in
> > + * hreg_compute_hflags_value. See the tables therein
> > + */
> > +static inline bool mmuidx_pr(int idx) { return !(idx & 1); }
> > +/*
> > + * If we want to use these macros for hash-style MMUs, we need to
> > + * add an if or another macro here.
> > + */
>
> Don't these work just fine for hash as well? Except for Booke.
Right, I think we want to restrict these to BookS mmus, but I don't
think we need to restrict to radix.
>
> > +static inline bool mmuidx_real(int idx) { return idx & 2; }
> > +static inline bool mmuidx_hv(int idx) { return idx & 4; }
> > +
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus
On Mon, Jun 14, 2021 at 04:16:29PM -0300, Bruno Larsen (billionai) wrote:
> Based-on: <20210518201146.794854-1-richard.hender...@linaro.org>
>
> This commit attempts to fix the first bug mentioned by Richard Henderson in
> https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06247.html
>
> To sumarize the bug here, when radix-style mmus are translating an
> address, they might need to call a second level of translation, with
> hypvervisor priviledges. However, the way it was being done up until
s/hypvervisor/hypervisor/
And... it's a super-common error, even amongst native speakers, but
s/priviledge/privilege/g
> this point meant that the second level translation had the same
> priviledges as the first level. This would only happen when a TCG guest
> was emulating KVM, which is why it hasn't been discovered yet.
>
> This patch attempts to correct that by making radix64_*_xlate functions
> receive the mmu_idx, and passing one with the correct permission for the
> second level translation.
>
> The mmuidx macros added by this patch are only correct for non-bookE
> mmus, because BookE style set the IS and DS bits inverted and there
> might be other subtle differences. However, there doesn't seem to be
> BookE cpus that have radix-style mmus, so we left a comment there to
> document the issue, in case a machine does have that and was missed.
Right, the mmuidx values are basically local to a cpu family. Radix
is only present on BookS 64-bit cpus.
> As part of this cleanup, we now need to send the correct mmmu_idx
> when calling get_phys_page_debug, otherwise we might not be able to see the
> memory that the CPU could
>
> Suggested-by: Richard Henderson
> Signed-off-by: Bruno Larsen (billionai)
> ---
> target/ppc/internal.h| 12
> target/ppc/mmu-radix64.c | 38 ++
> target/ppc/mmu-radix64.h | 2 +-
> target/ppc/mmu_helper.c | 8 +---
> 4 files changed, 40 insertions(+), 20 deletions(-)
>
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index f1fd3c8d04..003df7e8a9 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -245,4 +245,16 @@ static inline int prot_for_access_type(MMUAccessType
> access_type)
> g_assert_not_reached();
> }
>
> +/*
> + * These correspond to the mmu_idx values computed in
> + * hreg_compute_hflags_value. See the tables therein
> + */
> +static inline bool mmuidx_pr(int idx) { return !(idx & 1); }
> +/*
> + * If we want to use these macros for hash-style MMUs, we need to
> + * add an if or another macro here
I think move these to mmu-book3s-v3.h, since they're correct for both
the radix and hash sides of the modern book3s mmu.
.
> + */
> +static inline bool mmuidx_real(int idx) { return idx & 2; }
> +static inline bool mmuidx_hv(int idx) { return idx & 4; }
> +
> #endif /* PPC_INTERNAL_H */
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index cbd404bfa4..0ae8f6b572 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -155,7 +155,7 @@ static void ppc_radix64_raise_hsi(PowerPCCPU *cpu,
> MMUAccessType access_type,
>
> static bool ppc_radix64_check_prot(PowerPCCPU *cpu, MMUAccessType
> access_type,
> uint64_t pte, int *fault_cause, int *prot,
> - bool partition_scoped)
> + int mmu_idx, bool partition_scoped)
> {
> CPUPPCState *env = >env;
> int need_prot;
> @@ -173,7 +173,8 @@ static bool ppc_radix64_check_prot(PowerPCCPU *cpu,
> MMUAccessType access_type,
> /* Determine permissions allowed by Encoded Access Authority */
> if (!partition_scoped && (pte & R_PTE_EAA_PRIV) && msr_pr) {
> *prot = 0;
> -} else if (msr_pr || (pte & R_PTE_EAA_PRIV) || partition_scoped) {
> +} else if (mmuidx_pr(mmu_idx) || (pte & R_PTE_EAA_PRIV) ||
> + partition_scoped) {
> *prot = ppc_radix64_get_prot_eaa(pte);
> } else { /* !msr_pr && !(pte & R_PTE_EAA_PRIV) && !partition_scoped */
> *prot = ppc_radix64_get_prot_eaa(pte);
> @@ -299,7 +300,7 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU
> *cpu,
>ppc_v3_pate_t pate,
>hwaddr *h_raddr, int *h_prot,
>int *h_page_size, bool
> pde_addr,
> - bool guest_visible)
> + int mmu_idx, bool
> guest_visible)
> {
> int fault_cause = 0;
> hwaddr pte_addr;
> @@ -310,7 +311,9 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU
> *cpu,
> if (ppc_radix64_walk_tree(CPU(cpu)->as, g_raddr, pate.dw0 & PRTBE_R_RPDB,
>pate.dw0 & PRTBE_R_RPDS, h_raddr, h_page_size,
>, _cause, _addr) ||
> -
[RFC PATCH 7/8] spapr_numa, spapar_nvdimm: write secondary NUMA domain for nvdimms
Using the new 'device-node' property, write it in the nvdimm DT to set a
secondary domain for the persistent memory operation mode.
Note that this is only available in FORM2 affinity. FORM1 affinity
NVDIMMs aren't affected by this change.
CC: Shivaprasad G Bhat
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr_numa.c | 20
hw/ppc/spapr_nvdimm.c | 3 ++-
include/hw/ppc/spapr_numa.h | 3 +++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index b14dd543c8..0cabb6757f 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -266,6 +266,26 @@ void spapr_numa_write_associativity_dt(SpaprMachineState
*spapr, void *fdt,
sizeof(spapr->numa_assoc_array[nodeid];
}
+void spapr_numa_write_nvdimm_assoc_dt(SpaprMachineState *spapr, void *fdt,
+ int offset, int nodeid,
+ int device_node)
+{
+uint32_t *nvdimm_assoc_array = spapr->numa_assoc_array[nodeid];
+
+/*
+ * 'device-node' is the secondary domain for NVDIMMs when
+ * using FORM2. The secondary domain for FORM2 in QEMU
+ * is 0x3.
+ */
+if (spapr_ovec_test(spapr->ov5_cas, OV5_FORM2_AFFINITY)) {
+nvdimm_assoc_array[0x3] = cpu_to_be32(device_node);
+}
+
+_FDT((fdt_setprop(fdt, offset, "ibm,associativity",
+ nvdimm_assoc_array,
+ sizeof(spapr->numa_assoc_array[nodeid];
+}
+
static uint32_t *spapr_numa_get_vcpu_assoc(SpaprMachineState *spapr,
PowerPCCPU *cpu)
{
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index 91de1052f2..49a05736fe 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -142,7 +142,8 @@ static int spapr_dt_nvdimm(SpaprMachineState *spapr, void
*fdt,
_FDT((fdt_setprop_string(fdt, child_offset, "compatible", "ibm,pmemory")));
_FDT((fdt_setprop_string(fdt, child_offset, "device_type",
"ibm,pmemory")));
-spapr_numa_write_associativity_dt(spapr, fdt, child_offset, node);
+spapr_numa_write_nvdimm_assoc_dt(spapr, fdt, child_offset, node,
+ nvdimm->device_node);
buf = qemu_uuid_unparse_strdup(>uuid);
_FDT((fdt_setprop_string(fdt, child_offset, "ibm,unit-guid", buf)));
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index adaec8e163..af25741e70 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -26,6 +26,9 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr);
void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
int offset, int nodeid);
+void spapr_numa_write_nvdimm_assoc_dt(SpaprMachineState *spapr, void *fdt,
+ int offset, int nodeid,
+ int device_node);
int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
int offset, PowerPCCPU *cpu);
int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt,
--
2.31.1
[RFC PATCH 8/8] spapr: move memory/cpu less check to spapr_numa_FORM1_affinity_init()
FORM2 NUMA affinity is prepared to deal with memory/cpu less
NUMA nodes. This is used by the DAX KMEM driver to locate a
PAPR SCM device that has a different latency than the original
NUMA node from the regular memory.
Move this verification to FORM1 affinity code.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr.c | 33 -
hw/ppc/spapr_numa.c | 33 +
2 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c70b8b2f44..e42bfd314b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2772,39 +2772,6 @@ static void spapr_machine_init(MachineState *machine)
/* init CPUs */
spapr_init_cpus(spapr);
-/*
- * check we don't have a memory-less/cpu-less NUMA node
- * Firmware relies on the existing memory/cpu topology to provide the
- * NUMA topology to the kernel.
- * And the linux kernel needs to know the NUMA topology at start
- * to be able to hotplug CPUs later.
- */
-if (machine->numa_state->num_nodes) {
-for (i = 0; i < machine->numa_state->num_nodes; ++i) {
-/* check for memory-less node */
-if (machine->numa_state->nodes[i].node_mem == 0) {
-CPUState *cs;
-int found = 0;
-/* check for cpu-less node */
-CPU_FOREACH(cs) {
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-if (cpu->node_id == i) {
-found = 1;
-break;
-}
-}
-/* memory-less and cpu-less node */
-if (!found) {
-error_report(
- "Memory-less/cpu-less nodes are not supported (node
%d)",
- i);
-exit(1);
-}
-}
-}
-
-}
-
spapr->gpu_numa_id = spapr_numa_initial_nvgpu_numa_id(machine);
if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index 0cabb6757f..c7b0745455 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -162,6 +162,39 @@ static void
spapr_numa_FORM1_affinity_init(SpaprMachineState *spapr,
MachineState *machine)
{
bool using_legacy_numa = spapr_machine_using_legacy_numa(spapr);
+int i;
+
+/*
+ * check we don't have a memory-less/cpu-less NUMA node
+ * Firmware relies on the existing memory/cpu topology to provide the
+ * NUMA topology to the kernel.
+ * And the linux kernel needs to know the NUMA topology at start
+ * to be able to hotplug CPUs later.
+ */
+if (machine->numa_state->num_nodes) {
+for (i = 0; i < machine->numa_state->num_nodes; ++i) {
+/* check for memory-less node */
+if (machine->numa_state->nodes[i].node_mem == 0) {
+CPUState *cs;
+int found = 0;
+/* check for cpu-less node */
+CPU_FOREACH(cs) {
+PowerPCCPU *cpu = POWERPC_CPU(cs);
+if (cpu->node_id == i) {
+found = 1;
+break;
+}
+}
+/* memory-less and cpu-less node */
+if (!found) {
+error_report(
+ "Memory-less/cpu-less nodes are not supported (node
%d)",
+ i);
+exit(1);
+}
+}
+}
+}
/*
* Legacy NUMA guests (pseries-5.1 and older, or guests with only
--
2.31.1
[RFC PATCH 5/8] spapr: simplify spapr_numa_associativity_init params
When spapr_numa_associativity_init() was introduced it was being
called from spapr_machine_init(), where we have pointers to a
SpaprMachineState and a MachineState. Having both being passed
as params spared us from calling a macro to get the MachineState.
Previous patches moved the function away from spapr_machine_init(),
and the new locations doesn't have a pointer to MachineState ready.
This resulted in MACHINE(spapr) macro calls as the second parameter
in both callers.
Simplify the function by folding the 'MACHINE(spapr)' macro into the
function body, which can now receive only a pointer to
SpaprMachineState.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_numa.c | 4 ++--
include/hw/ppc/spapr_numa.h | 3 +--
4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c6def3690a..c70b8b2f44 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1784,7 +1784,7 @@ static int spapr_post_load(void *opaque, int version_id)
* initialized and hotplug operations won't fail in both before
* and after CAS migration cases.
*/
- spapr_numa_associativity_init(spapr, MACHINE(spapr));
+ spapr_numa_associativity_init(spapr);
return err;
}
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6d6ffcc92b..73aca0aed6 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1199,7 +1199,7 @@ target_ulong do_client_architecture_support(PowerPCCPU
*cpu,
spapr_ovec_cleanup(ov1_guest);
/* Init numa_assoc_array */
-spapr_numa_associativity_init(spapr, MACHINE(spapr));
+spapr_numa_associativity_init(spapr);
/*
* Ensure the guest asks for an interrupt mode we support;
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index c8fd66b53a..b14dd543c8 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -181,9 +181,9 @@ static void
spapr_numa_FORM1_affinity_init(SpaprMachineState *spapr,
spapr_numa_define_associativity_domains(spapr);
}
-void spapr_numa_associativity_init(SpaprMachineState *spapr,
- MachineState *machine)
+void spapr_numa_associativity_init(SpaprMachineState *spapr)
{
+MachineState *machine = MACHINE(spapr);
SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
int nb_numa_nodes = machine->numa_state->num_nodes;
int i, j, max_nodes_with_gpus;
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index 6f9f02d3de..adaec8e163 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -22,8 +22,7 @@
* function. spapr_machine_init() is the only caller for it, and
* it has both pointers resolved already.
*/
-void spapr_numa_associativity_init(SpaprMachineState *spapr,
- MachineState *machine);
+void spapr_numa_associativity_init(SpaprMachineState *spapr);
void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
int offset, int nodeid);
--
2.31.1
[RFC PATCH 6/8] nvdimm: add PPC64 'device-node' property
The spapr-nvdimm driver is able to operate in two ways. The first
one is as a regular memory in which the NUMA node of the parent
pc-dimm class is used. The second mode, as persistent memory, allows for
a different NUMA node to be used based on the locality of the device.
At this moment we don't have a way to express this second NUMA node for
the persistent memory mode. This patch introduces a new 'device-node'
property that will be used by the PPC64 spapr-nvdimm driver to set a
second NUMA node for the nvdimm.
CC: Shivaprasad G Bhat
CC: Igor Mammedov
Signed-off-by: Daniel Henrique Barboza
---
hw/mem/nvdimm.c | 28
include/hw/mem/nvdimm.h | 12
2 files changed, 40 insertions(+)
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 7397b67156..030ccf7575 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -96,6 +96,29 @@ static void nvdimm_set_uuid(Object *obj, Visitor *v, const
char *name,
g_free(value);
}
+static void nvdimm_get_device_node(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+NVDIMMDevice *nvdimm = NVDIMM(obj);
+uint8_t value = nvdimm->device_node;
+
+visit_type_uint8(v, name, , errp);
+}
+
+static void nvdimm_set_device_node(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+NVDIMMDevice *nvdimm = NVDIMM(obj);
+uint8_t value;
+
+if (!visit_type_uint8(v, name, , errp)) {
+return;
+}
+
+nvdimm->device_node = value;
+}
static void nvdimm_init(Object *obj)
{
@@ -105,6 +128,11 @@ static void nvdimm_init(Object *obj)
object_property_add(obj, NVDIMM_UUID_PROP, "QemuUUID", nvdimm_get_uuid,
nvdimm_set_uuid, NULL, NULL);
+
+object_property_add(obj, NVDIMM_DEVICE_NODE, "uint8",
+nvdimm_get_device_node,
+nvdimm_set_device_node,
+NULL, NULL);
}
static void nvdimm_finalize(Object *obj)
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index bcf62f825c..430169322f 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -49,6 +49,7 @@
OBJECT_DECLARE_TYPE(NVDIMMDevice, NVDIMMClass, NVDIMM)
#define NVDIMM_LABEL_SIZE_PROP "label-size"
+#define NVDIMM_DEVICE_NODE "device-node"
#define NVDIMM_UUID_PROP "uuid"
#define NVDIMM_UNARMED_PROP"unarmed"
@@ -89,6 +90,17 @@ struct NVDIMMDevice {
* The PPC64 - spapr requires each nvdimm device have a uuid.
*/
QemuUUID uuid;
+
+ /*
+* The spapr-nvdimm (PPC64 NVDIMM) driver supports two modes of
+* operation: regular memory and persistent memory. When using the
+* device as regular memory, the NUMA nodeid that comes from
+* PC_DIMM_NODEPROP is to be used. When used as persistent memory,
+* the guest should consider the 'device-node' instead since it
+* indicates the locality of the device to an established NUMA
+* node, which is more relevant to this type of usage.
+*/
+uint8_t device_node;
};
struct NVDIMMClass {
--
2.31.1
[RFC PATCH 2/8] spapr_numa.c: split FORM1 code into helpers
The upcoming FORM2 NUMA affinity will support asymmetric NUMA topologies
and doesn't need be concerned with all the legacy support for older
pseries FORM1 guests.
We're also not going to calculate associativity domains based on numa
distance (via spapr_numa_define_associativity_domains) since the
distances will be written directly into new DT properties.
Let's split FORM1 code into its own functions to allow for easier
insertion of FORM2 logic later on.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr_numa.c | 61 +
1 file changed, 39 insertions(+), 22 deletions(-)
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index 779f18b994..04a86f9b5b 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -155,6 +155,32 @@ static void
spapr_numa_define_associativity_domains(SpaprMachineState *spapr)
}
+/*
+ * Set NUMA machine state data based on FORM1 affinity semantics.
+ */
+static void spapr_numa_FORM1_affinity_init(SpaprMachineState *spapr,
+ MachineState *machine)
+{
+bool using_legacy_numa = spapr_machine_using_legacy_numa(spapr);
+
+/*
+ * Legacy NUMA guests (pseries-5.1 and older, or guests with only
+ * 1 NUMA node) will not benefit from anything we're going to do
+ * after this point.
+ */
+if (using_legacy_numa) {
+return;
+}
+
+if (!spapr_numa_is_symmetrical(machine)) {
+error_report("Asymmetrical NUMA topologies aren't supported "
+ "in the pSeries machine");
+exit(EXIT_FAILURE);
+}
+
+spapr_numa_define_associativity_domains(spapr);
+}
+
void spapr_numa_associativity_init(SpaprMachineState *spapr,
MachineState *machine)
{
@@ -210,22 +236,7 @@ void spapr_numa_associativity_init(SpaprMachineState
*spapr,
spapr->numa_assoc_array[i][MAX_DISTANCE_REF_POINTS] = cpu_to_be32(i);
}
-/*
- * Legacy NUMA guests (pseries-5.1 and older, or guests with only
- * 1 NUMA node) will not benefit from anything we're going to do
- * after this point.
- */
-if (using_legacy_numa) {
-return;
-}
-
-if (!spapr_numa_is_symmetrical(machine)) {
-error_report("Asymmetrical NUMA topologies aren't supported "
- "in the pSeries machine");
-exit(EXIT_FAILURE);
-}
-
-spapr_numa_define_associativity_domains(spapr);
+spapr_numa_FORM1_affinity_init(spapr, machine);
}
void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
@@ -302,12 +313,8 @@ int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState
*spapr, void *fdt,
return ret;
}
-/*
- * Helper that writes ibm,associativity-reference-points and
- * max-associativity-domains in the RTAS pointed by @rtas
- * in the DT @fdt.
- */
-void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas)
+static void spapr_numa_FORM1_write_rtas_dt(SpaprMachineState *spapr,
+ void *fdt, int rtas)
{
MachineState *ms = MACHINE(spapr);
SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
@@ -365,6 +372,16 @@ void spapr_numa_write_rtas_dt(SpaprMachineState *spapr,
void *fdt, int rtas)
maxdomains, sizeof(maxdomains)));
}
+/*
+ * Helper that writes ibm,associativity-reference-points and
+ * max-associativity-domains in the RTAS pointed by @rtas
+ * in the DT @fdt.
+ */
+void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas)
+{
+spapr_numa_FORM1_write_rtas_dt(spapr, fdt, rtas);
+}
+
static target_ulong h_home_node_associativity(PowerPCCPU *cpu,
SpaprMachineState *spapr,
target_ulong opcode,
--
2.31.1
[RFC PATCH 3/8] spapr_numa.c: wait for CAS before writing rtas DT
spapr_numa_write_rtas_dt() is called from spapr_dt_rtas(), which in
turned is called by spapr_build_fdt(). spapr_build_fdt() is called in
two places: spapr_machine_reset() and do_client_architecture_support().
When called in machine_reset() we're writing RTAS nodes with NUMA
artifacts without going through CAS first.
This is not an issue because we always write the same thing in DT, since
we support just FORM1 NUMA affinity. With the upcoming FORM2 support,
we're now reliant on guest choice to decide what to write.
Instead of taking a guess (e.g. default to FORM1, switch to FORM2 if
guest chooses it), postpone the writing of
ibm,associativity-reference-points and ibm,max-associativity-domains
until we're sure what was negotiated with the guest.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr_numa.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index 04a86f9b5b..e1a7f80076 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -379,6 +379,10 @@ static void
spapr_numa_FORM1_write_rtas_dt(SpaprMachineState *spapr,
*/
void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas)
{
+if (spapr_ovec_empty(spapr->ov5_cas)) {
+return;
+}
+
spapr_numa_FORM1_write_rtas_dt(spapr, fdt, rtas);
}
--
2.31.1
[RFC PATCH 4/8] spapr_numa.c: base FORM2 NUMA affinity support
The main feature of FORM2 affinity support is the separation of NUMA
distances from ibm,associativity information. This allows for a more
flexible and straightforward NUMA distance assignment without relying on
complex associations between several levels of NUMA via
ibm,associativity matches.
Another feature is its extensibility. This base support contains the
facilities for NUMA distance assignment. In the future more facilities
will be added for latency, performance, bandwidth and so on.
This patch implements the base FORM2 affinity support as follows:
- the use of FORM2 associativity is indicated by using bit 2 of byte 5
of ibm,architecture-vec-5. A FORM2 aware guest can choose to use FORM1
or FORM2 affinity. Setting both forms will default to FORM2. We rely on
CAS to decide what to write in the DT, so all writing is postponed until
client architecture support is done;
- ibm,associativity-reference-points has a new semantic. Instead of
being used to calculate distances via NUMA levels, the concept of
subdomain was introduced. The primary domain is the first element of the
array, secondary domain is the second element and so on. At this moment,
the only usage of this semantic is with NVDIMMs. This will be explained
further in the next patches;
- two new RTAS DT artifacts are introduced: ibm,numa-lookup-index-table
and ibm,numa-distance-table. The index table is used to list all the
NUMA logical domains of the platform, in ascending order, and allows for
spartial NUMA configurations (although QEMU ATM doesn't support that).
ibm,numa-distance-table is an array that contains all the distances from
the first NUMA node to all other nodes, then the second NUMA node
distances to all other nodes and so on;
- spapr_post_load changes: since we're adding a new NUMA affinity that
isn't compatible with the existing one, migration must be handled
accordingly because we can't be certain of whether the guest went
through CAS in the source. The solution chosen is to initiate the NUMA
associativity data in spapr_post_load() unconditionally. The worst case
would be to write the DT twice if the guest is in pre-CAS stage.
Otherwise, we're making sure that a FORM1 guest will have the
spapr->numa_assoc_array initialized with the proper information based on
user distance, something that we're not doing with FORM2.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr.c | 17 +
hw/ppc/spapr_numa.c | 140 +++-
include/hw/ppc/spapr_ovec.h | 1 +
3 files changed, 157 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index dd57b30cff..c6def3690a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1770,6 +1770,22 @@ static int spapr_post_load(void *opaque, int version_id)
return err;
}
+/*
+ * NUMA data init is made in CAS time. There is no reliable
+ * way of telling whether the guest already went through CAS
+ * in the source due to how spapr_ov5_cas_needed works: a
+ * FORM1 guest can be migrated with ov5_cas empty regardless
+ * of going through CAS first.
+ *
+ * One solution is to always call numa_associativity_init. The
+ * downside is that a guest migrated before CAS will run
+ * numa_associativity_init again when going through it, but
+ * at least we're making sure spapr->numa_assoc_array will be
+ * initialized and hotplug operations won't fail in both before
+ * and after CAS migration cases.
+ */
+ spapr_numa_associativity_init(spapr, MACHINE(spapr));
+
return err;
}
@@ -2733,6 +2749,7 @@ static void spapr_machine_init(MachineState *machine)
}
spapr_ovec_set(spapr->ov5, OV5_FORM1_AFFINITY);
+spapr_ovec_set(spapr->ov5, OV5_FORM2_AFFINITY);
/* advertise support for dedicated HP event source to guests */
if (spapr->use_hotplug_event_source) {
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index e1a7f80076..c8fd66b53a 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -202,6 +202,16 @@ void spapr_numa_associativity_init(SpaprMachineState
*spapr,
spapr->numa_assoc_array[i][0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS);
spapr->numa_assoc_array[i][MAX_DISTANCE_REF_POINTS] = cpu_to_be32(i);
+/*
+ * For FORM2 affinity the initialization above is enough. No
+ * need to fill non-zero NUMA nodes with node_id because
+ * there is no associativity domain match to calculate
+ * NUMA distances in FORM2.
+ */
+if (spapr_ovec_test(spapr->ov5_cas, OV5_FORM2_AFFINITY)) {
+continue;
+}
+
/*
* Fill all associativity domains of non-zero NUMA nodes with
* node_id. This is required because the default value (0) is
@@ -236,7 +246,16 @@ void spapr_numa_associativity_init(SpaprMachineState
*spapr,
spapr->numa_assoc_array[i][MAX_DISTANCE_REF_POINTS] = cpu_to_be32(i);
}
-
[RFC PATCH 1/8] spapr: move NUMA data init to do_client_architecture_support()
The pSeries machine will support a new NUMA affinity form, FORM2.
This new FORM will be negotiated via ibm,architecture-vec5 during
CAS. All artifacts and assumptions that are currently on use for
FORM1 affinity will be deprecated in a guest that chooses to use
FORM2.
This means that we're going to wait until CAS to determine whether
we're going to use FORM1 and FORM2. This patch starts doing that
by moving spapr_numa_associativity_init() from spapr_machine_init()
to do_client_architecture_support().
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr.c | 3 ---
hw/ppc/spapr_hcall.c | 4
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4dd90b75cc..dd57b30cff 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2790,9 +2790,6 @@ static void spapr_machine_init(MachineState *machine)
spapr->gpu_numa_id = spapr_numa_initial_nvgpu_numa_id(machine);
-/* Init numa_assoc_array */
-spapr_numa_associativity_init(spapr, machine);
-
if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
spapr->max_compat_pvr)) {
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f25014afda..6d6ffcc92b 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -11,6 +11,7 @@
#include "helper_regs.h"
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_cpu_core.h"
+#include "hw/ppc/spapr_numa.h"
#include "mmu-hash64.h"
#include "cpu-models.h"
#include "trace.h"
@@ -1197,6 +1198,9 @@ target_ulong do_client_architecture_support(PowerPCCPU
*cpu,
spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
spapr_ovec_cleanup(ov1_guest);
+/* Init numa_assoc_array */
+spapr_numa_associativity_init(spapr, MACHINE(spapr));
+
/*
* Ensure the guest asks for an interrupt mode we support;
* otherwise terminate the boot.
--
2.31.1
[RFC PATCH 0/8] pSeries base FORM2 NUMA affinity support
Hi, This RFC series implements FORM2 NUMA associativity support in the pSeries machine. This new associativity format is going to be added in the LOPAR spec in the near future. For now, the preview of the specification can be found in Aneesh kernel side patches that implements this support, specially the documentation patch [2]. For QEMU, the most drastic change FORM2 brings is that, at long last, we're free from the shackles of an overcomplicated and bloated way of calculating NUMA distances. This new affinity format promotes separation from performance metrics such as distance, latency, bandwidth and so on from the ibm,associativity arrays of the devices. This also allows for asymmetric NUMA configurations. FORM2 is set by ibm,architecture-vec-5 bit 2 byte 5. This means that the guest is able to choose between FORM1 and FORM2 during CAS, and we need to adapt NUMA internals accordingly based on this choice. Patches 1 to 5 implement the base FORM2 support in the pSeries machine. Patches 6-8 deal with NVDIMM changes. FORM2 allows NVDIMMs to declare an extra NUMA node called 'device-node' to support their use as persistent memory. 'device-node' is locality based an can be different from the NUMA node that the NVDIMM belongs to when used as regular memory. With this series and Aneesh's guest kernel from [1], this is the 'numactl -H' output of this guest: - sudo ppc64-softmmu/qemu-system-ppc64 \ -machine pseries,accel=kvm,usb=off,dump-guest-core=off \ -m size=14G,slots=256,maxmem=256G -smp 8,maxcpus=8,cores=2,threads=2,sockets=2 \ (...) -object memory-backend-ram,id=mem0,size=4G -numa node,memdev=mem0,cpus=0-1,nodeid=0 \ -object memory-backend-ram,id=mem1,size=4G -numa node,memdev=mem1,cpus=2-3,nodeid=1 \ -object memory-backend-ram,id=mem2,size=4G -numa node,memdev=mem2,cpus=4-5,nodeid=2 \ -object memory-backend-ram,id=mem3,size=2G -numa node,memdev=mem3,cpus=6-7,nodeid=3 \ -numa dist,src=0,dst=1,val=22 -numa dist,src=0,dst=2,val=22 -numa dist,src=0,dst=3,val=22 \ -numa dist,src=1,dst=0,val=44 -numa dist,src=1,dst=2,val=44 -numa dist,src=1,dst=3,val=44 \ -numa dist,src=2,dst=0,val=66 -numa dist,src=2,dst=1,val=66 -numa dist,src=2,dst=3,val=66 \ -numa dist,src=3,dst=0,val=88 -numa dist,src=3,dst=1,val=88 -numa dist,src=3,dst=2,val=88 # numactl -H available: 4 nodes (0-3) node 0 cpus: 0 1 node 0 size: 3987 MB node 0 free: 3394 MB node 1 cpus: 2 3 node 1 size: 4090 MB node 1 free: 4073 MB node 2 cpus: 4 5 node 2 size: 4090 MB node 2 free: 4072 MB node 3 cpus: 6 7 node 3 size: 2027 MB node 3 free: 2012 MB node distances: node 0 1 2 3 0: 10 22 22 22 1: 44 10 44 44 2: 66 66 10 66 3: 88 88 88 10 The exact user NUMA distances were reflected in the kernel, without any approximation like we have to do for FORM1. [1] https://lore.kernel.org/linuxppc-dev/20210614164003.196094-1-aneesh.ku...@linux.ibm.com/ [2] https://lore.kernel.org/linuxppc-dev/20210614164003.196094-8-aneesh.ku...@linux.ibm.com/ Daniel Henrique Barboza (8): spapr: move NUMA data init to do_client_architecture_support() spapr_numa.c: split FORM1 code into helpers spapr_numa.c: wait for CAS before writing rtas DT spapr_numa.c: base FORM2 NUMA affinity support spapr: simplify spapr_numa_associativity_init params nvdimm: add PPC64 'device-node' property spapr_numa, spapar_nvdimm: write secondary NUMA domain for nvdimms spapr: move memory/cpu less check to spapr_numa_FORM1_affinity_init() hw/mem/nvdimm.c | 28 hw/ppc/spapr.c | 53 +++- hw/ppc/spapr_hcall.c| 4 + hw/ppc/spapr_numa.c | 250 +--- hw/ppc/spapr_nvdimm.c | 3 +- include/hw/mem/nvdimm.h | 12 ++ include/hw/ppc/spapr_numa.h | 6 +- include/hw/ppc/spapr_ovec.h | 1 + 8 files changed, 299 insertions(+), 58 deletions(-) -- 2.31.1
Re: [PATCH 0/4] modules: add support for target-specific modules.
N¬Æ¦º[b¥ªíë,j¢Â ú+«g{Oj»vëß:ëâè4f
íz{S©ì}êÄÊ x*º^vâÖà¨×§µ<©z×±·úej)Ü
ªìz
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH v2 0/1] PALcode fixes for guest OS console initialization
> On Jun 13, 2021, at 2:09 PM, Jason Thorpe wrote: > > but there is still some research going on about how > real DEC SRM initializes a particular field in various circumstances; > this is my best guess based on available documentation and observed > behavior of real machines, and is sufficient for the BSD operating systems. FWIW, I have since confirmed my hypothesis about how genuine DEC SRM fills in the "type" and "term_type" fields on 2 different generations of PCI-based Alpha systems (i.e. it matches the patch submitted here). -- thorpej
[Bug 1913668] Re: FPE in npcm7xx_pwm_calculate_freq
This no longer reproduces for me. ** Changed in: qemu Status: Confirmed => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1913668 Title: FPE in npcm7xx_pwm_calculate_freq Status in QEMU: Incomplete Bug description: Reproducer: cat << EOF | ./qemu-system-aarch64 -M npcm750-evb \ -accel qtest -qtest stdio write 0xf0103008 0x4 0x0900 write 0xf010300c 0x4 0x EOF Trace: ../hw/misc/npcm7xx_pwm.c:94:17: runtime error: division by zero SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../hw/misc/npcm7xx_pwm.c:94:17 in AddressSanitizer:DEADLYSIGNAL = ==717868==ERROR: AddressSanitizer: FPE on unknown address 0x5597c7190150 (pc 0x5597c7190150 bp 0x7fffcb17c5d0 sp 0x7fffcb17c4e0 T0) #0 0x5597c7190150 in npcm7xx_pwm_calculate_freq /hw/misc/npcm7xx_pwm.c:94:17 #1 0x5597c7190150 in npcm7xx_pwm_update_freq /hw/misc/npcm7xx_pwm.c:122:21 #2 0x5597c718f06d in npcm7xx_pwm_write /hw/misc/npcm7xx_pwm.c #3 0x5597c8d241fe in memory_region_write_accessor /softmmu/memory.c:491:5 #4 0x5597c8d23bfb in access_with_adjusted_size /softmmu/memory.c:552:18 #5 0x5597c8d23467 in memory_region_dispatch_write /softmmu/memory.c #6 0x5597c90b3ffb in flatview_write_continue /softmmu/physmem.c:2759:23 #7 0x5597c90a971b in flatview_write /softmmu/physmem.c:2799:14 #8 0x5597c90a971b in address_space_write /softmmu/physmem.c:2891:18 #9 0x5597c8d11eee in qtest_process_command /softmmu/qtest.c:539:13 #10 0x5597c8d0eb97 in qtest_process_inbuf /softmmu/qtest.c:797:9 #11 0x5597c955f286 in fd_chr_read /chardev/char-fd.c:68:9 #12 0x7f994c124aae in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51aae) #13 0x5597c9bba363 in glib_pollfds_poll /util/main-loop.c:232:9 #14 0x5597c9bba363 in os_host_main_loop_wait /util/main-loop.c:255:5 #15 0x5597c9bba363 in main_loop_wait /util/main-loop.c:531:11 #16 0x5597c8c75599 in qemu_main_loop /softmmu/runstate.c:721:9 #17 0x5597c6f021fd in main /softmmu/main.c:50:5 #18 0x7f994bbc9cc9 in __libc_start_main csu/../csu/libc-start.c:308:16 #19 0x5597c6e55bc9 in _start (/home/alxndr/Development/qemu/build/qemu-system-aarch64+0x3350bc9) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1913668/+subscriptions
[Bug 1911216] Re: abort issue locates in hw/usb/hcd-ohci.c:1297:ohci_frame_boundary
OSS-Fuzz still has a functioning reproducer. I'll copy this one over to gitlab -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1911216 Title: abort issue locates in hw/usb/hcd-ohci.c:1297:ohci_frame_boundary Status in QEMU: Incomplete Bug description: Hello, I found an assertion failure in hw/usb/hcd-ohci.c:1297 This was found in latest version 5.2.0. my reproduced environment is as follows: Host: ubuntu 18.04 Guest: ubuntu 18.04 QEMU boot command line: qemu-system-x86_64 -enable-kvm -boot c -m 4G -drive format=qcow2,file=./ubuntu.img -nic user,hostfwd=tcp:0.0.0.0:-:22 -display none -device pci-ohci,id=ohci -device usb-tablet,bus=ohci.0,port=1,id=usbdev1 backtrace is as follows pwndbg> bt #0 0x7fdf392aa438 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #1 0x7fdf392ac03a in __GI_abort () at abort.c:89 #2 0x55c613721118 in ohci_frame_boundary (opaque=0x627191f0) at hw/usb/hcd-ohci.c:1297 #3 0x55c6140bdf0e in timerlist_run_timers (timer_list=0x60b5bcc0) at util/qemu-timer.c:572 #4 0x55c6140be15a in qemu_clock_run_timers (type=QEMU_CLOCK_VIRTUAL) at util/qemu-timer.c:586 #5 0x55c6140beac7 in qemu_clock_run_all_timers () at util/qemu-timer.c:672 #6 0x55c6140a1938 in main_loop_wait (nonblocking=0) at util/main-loop.c:523 #7 0x55c6125d87e9 in qemu_main_loop () at /home/dell/qemu5-hypervisor/vm/fuzz-seedpool/hcd-ohci/qemu-5.1.0/softmmu/vl.c:1676 #8 0x55c613f216ea in main (argc=7, argv=0x7fff174cdd28, envp=0x7fff174cdd68) at /home/dell/qemu5-hypervisor/vm/fuzz-seedpool/hcd-ohci/qemu-5.1.0/softmmu/main.c:49 #9 0x7fdf39295840 in __libc_start_main (main=0x55c613f21699 , argc=7, argv=0x7fff174cdd28, init=, fini=, rtld_fini=, stack_end=0x7fff174cdd18) at ../csu/libc-start.c:291 #10 0x55c6120a4349 in _start () The poc is attached. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1911216/+subscriptions
Re: [PATCH v4 0/2] Gitlab: Add issue templates
On 6/7/21 11:31 AM, John Snow wrote: Add "Bug" and "Feature Request" templates to the Gitlab interface to help improve the quality of newly reported issues. To see what this looks like, I've temporarily allowed my Gitlab fork to diverge with these files merged. See my fork's "new issue" page to see it in action: https://gitlab.com/jsnow/qemu/-/issues/new?issue (It's outdated a bit for V4, but you get the idea.) These patches do not add a "default" template, the user still has to select one from the list. I recommend that someone with permissions updates the default template: 1. https://gitlab.com/qemu-project/qemu/edit 2. ctrl+f "Default description template for issues" 3. Update the default to the (suggested) below: ``` ``` We can use this cover letter to discuss/review the wording on that default template which exists outside of repository data. V4: - Change the "build on master" to be more of a nudge than a mandate, with improved instructions (stefanha, danpb) V3: - Add pointer to https://www.qemu.org/download/#source - Add pointer to https://www.qemu.org/contribute/security-process/ - Remove blurb covering tracing instructions. V2: - Updated both templates based on feedback from Peter, Daniel, and Thomas. John Snow (2): GitLab: Add "Bug" issue reporting template GitLab: Add "Feature Request" issue template. .gitlab/issue_templates/bug.md | 64 ++ .gitlab/issue_templates/feature_request.md | 32 +++ 2 files changed, 96 insertions(+) create mode 100644 .gitlab/issue_templates/bug.md create mode 100644 .gitlab/issue_templates/feature_request.md Gentle ping. I do not know whose jurisdiction this falls under. --js
[Bug 1892761] Re: Heap-use-after-free through double-fetch in ehci
No. If we figure out some way to consistently reproduce double-fetches in a non-fuzzer build, I'll report the issue again, but this can probably be closed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892761 Title: Heap-use-after-free through double-fetch in ehci Status in QEMU: Incomplete Bug description: Hello, I don't have a qtest reproducer for this crash because it involves a DMA double-fetch, and I don't think we can reproduce those with qtest. Instead, I attached the pseudo-qtest trace produced by the fuzzer, along with some trace events. The lines annotated with [DMA] are write commands that were triggered by a callback from a DMA read by the device. The lines annotated with [DOUBLE-FETCH] are DMA accesses that hit the same address more than once (possible double-fetches). I am still thinking of nicer ways of presenting this trace and providing a reproducer. -Alex To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1892761/+subscriptions
[Bug 1892962] Re: Segfault in usb_bus_from_device
OSS-Fuzz never came across this one. Probably fixed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892962 Title: Segfault in usb_bus_from_device Status in QEMU: Incomplete Bug description: Hello, Reproducer: cat << EOF | ./qemu-system-i386 -machine q35 \ -device ich9-usb-ehci1,bus=pcie.0,addr=1d.7,\ multifunction=on,id=ich9-ehci-1 \ -device ich9-usb-uhci1,bus=pcie.0,addr=1d.0,\ multifunction=on,masterbus=ich9-ehci-1.0,firstport=0 \ -device usb-tablet,bus=ich9-ehci-1.0,port=1,usb_version=1 \ -display none -nodefaults -qtest stdio -accel qtest outl 0xcf8 0x8000e803 outl 0xcfc 0xff00ff00 outl 0xcf8 0x8000e821 outb 0xcfc 0xff outl 0xff10 0x8500057e clock_step clock_step outb 0xff00 0x49 write 0x2 0x1 0x40 write 0x46 0x1 0xfb write 0x48 0x1 0x2d write 0x4a 0x1 0xe0 write 0x4c 0x1 0x16 write 0x4e 0x1 0xfa write 0xfa001c 0x1 0x04 clock_step write 0x46 0x1 0xfb write 0xfa001d 0x1 0xff clock_step write 0x8 0x1 0xe0 write 0xa 0x1 0x16 write 0x1600e6 0x1 0x9c write 0x1600e8 0x1 0xe1 write 0x1600eb 0x1 0x30 clock_step clock_step write 0x10 0x1 0xe0 write 0x12 0x1 0x16 write 0x1600e6 0x1 0x9c write 0x6 0x1 0x9c write 0x8 0x1 0xe1 write 0xa 0x1 0x40 write 0xb 0x1 0x30 clock_step write 0x14 0x1 0xe0 write 0x16 0x1 0x16 write 0x1600e6 0x1 0x9c write 0x6 0x1 0x9c clock_step write 0x18 0x1 0xe0 write 0x1a 0x1 0x16 write 0x1600e6 0x1 0x9c write 0x6 0x1 0x9c clock_step write 0x1c 0x1 0xe0 write 0x1e 0x1 0x16 write 0x1600e6 0x1 0x9c write 0x6 0x1 0x9c clock_step write 0x20 0x1 0xe0 write 0x22 0x1 0x16 write 0x1600e6 0x1 0x9c write 0x6 0x1 0x9c clock_step EOF The trace: ... [S +0.087589] OK [R +0.087596] write 0x1600e6 0x1 0x9c OK [S +0.087603] OK [R +0.087655] write 0x6 0x1 0x9c OK [S +0.087667] OK [R +0.087675] clock_step 784168@1598406646.189133:usb_uhci_frame_start nr 8 784168@1598406646.189141:usb_uhci_td_load qh 0x0, td 0x1600e0, ctrl 0x9c0180, token 0x30e1 784168@1598406646.189147:usb_uhci_packet_add token 0x0, td 0x1600e0 784168@1598406646.189151:usb_packet_state_change bus 0, port 1, ep 0, packet 0x61143c00, state undef -> setup 784168@1598406646.189161:usb_packet_state_change bus 0, port 1, ep 0, packet 0x61143c00, state setup -> complete 784168@1598406646.189165:usb_uhci_packet_complete_success token 0x0, td 0x1600e0 784168@1598406646.189168:usb_uhci_packet_del token 0x0, td 0x1600e0 784168@1598406646.189174:usb_uhci_td_complete qh 0x0, td 0x1600e0 784168@1598406646.189179:usb_uhci_td_load qh 0x0, td 0x0, ctrl 0x9c0182, token 0x304000e1 784168@1598406646.189183:usb_uhci_packet_add token 0x0, td 0x0 784168@1598406646.189187:usb_packet_state_change bus 0, port 1, ep 0, packet 0x61143d40, state undef -> setup /home/alxndr/Development/qemu/general-fuzz/include/hw/usb.h:526:12: runtime error: member access within null pointer of type 'USBDevice' (aka 'struct USBDevice') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/alxndr/Development/qemu/general-fuzz/include/hw/usb.h:526:12 in /home/alxndr/Development/qemu/general-fuzz/include/hw/usb.h:526:12: runtime error: member access within null pointer of type 'DeviceState' (aka 'struct DeviceState') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/alxndr/Development/qemu/general-fuzz/include/hw/usb.h:526:12 in AddressSanitizer:DEADLYSIGNAL = ==784168==ERROR: AddressSanitizer: SEGV on unknown address 0x0050 (pc 0x5599c43df445 bp 0x7ffec2833e50 sp 0x7ffec2833dc0 T0) ==784168==The signal is caused by a READ memory access. ==784168==Hint: address points to the zero page. #0 0x5599c43df445 in usb_bus_from_device /home/alxndr/Development/qemu/general-fuzz/include/hw/usb.h:526:12 #1 0x5599c43ea95c in usb_packet_set_state /home/alxndr/Development/qemu/general-fuzz/build/../hw/usb/core.c:549:23 #2 0x5599c43e8abd in usb_handle_packet /home/alxndr/Development/qemu/general-fuzz/build/../hw/usb/core.c:438:17 #3 0x5599c4b02497 in uhci_handle_td /home/alxndr/Development/qemu/general-fuzz/build/../hw/usb/hcd-uhci.c:892:9 #4 0x5599c4afbd26 in uhci_process_frame /home/alxndr/Development/qemu/general-fuzz/build/../hw/usb/hcd-uhci.c:1075:15 #5 0x5599c4aed2e3 in uhci_frame_timer /home/alxndr/Development/qemu/general-fuzz/build/../hw/usb/hcd-uhci.c:1174:9 #6 0x5599c7620917 in timerlist_run_timers /home/alxndr/Development/qemu/general-fuzz/build/../util/qemu-timer.c:572:9 #7 0x5599c7620e51 in qemu_clock_run_timers /home/alxndr/Development/qemu/general-fuzz/build/../util/qemu-timer.c:586:12 #8 0x5599c5f35a13 in qtest_clock_warp
[Bug 1888606] Re: Heap-use-after-free in virtio_gpu_ctrl_response
OSS-Fuzz says it was fixed some months ago, and it has not found a reproducer since. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1888606 Title: Heap-use-after-free in virtio_gpu_ctrl_response Status in QEMU: Incomplete Bug description: Hello, Here is a reproducer (build with --enable-sanitizers): cat << EOF | ./i386-softmmu/qemu-system-i386 -nographic -M pc -nodefaults -m 512M -device virtio-vga -qtest stdio outl 0xcf8 0x80001018 outl 0xcfc 0xe080 outl 0xcf8 0x80001020 outl 0xcf8 0x80001004 outw 0xcfc 0x7 writeq 0xe0801024 0x10646c00776c6cff writeq 0xe080102d 0xe080100032 writeq 0xe0801015 0x12b2901ba00 write 0x10646c02 0x1 0x2c write 0x999 0x1 0x25 write 0x8 0x1 0x78 write 0x2c7 0x1 0x32 write 0x2cb 0x1 0xff write 0x2cc 0x1 0x7e writeq 0xe0803000 0xf2b8f0540ff83 EOF The ASAN trace: ==29798==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d050e8 at pc 0x560629814761 bp 0x7ffe916eb1e0 sp 0x7ffe916eb1d8 READ of size 8 at 0x60d050e8 thread T0 #0 0x560629814760 in virtio_gpu_ctrl_response /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:181:42 #1 0x56062981adc8 in virtio_gpu_ctrl_response_nodata /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:193:5 #2 0x56062981adc8 in virtio_gpu_simple_process_cmd /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:791:9 #3 0x5606298175f8 in virtio_gpu_process_cmdq /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:820:9 #4 0x56062a8f1c96 in aio_bh_poll /home/alxndr/Development/qemu/util/async.c:164:13 #5 0x56062a887b9d in aio_dispatch /home/alxndr/Development/qemu/util/aio-posix.c:380:5 #6 0x56062a8f6b1c in aio_ctx_dispatch /home/alxndr/Development/qemu/util/async.c:306:5 #7 0x7f0d5e1cf9ed in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed) #8 0x56062a919571 in glib_pollfds_poll /home/alxndr/Development/qemu/util/main-loop.c:217:9 #9 0x56062a919571 in os_host_main_loop_wait /home/alxndr/Development/qemu/util/main-loop.c:240:5 #10 0x56062a919571 in main_loop_wait /home/alxndr/Development/qemu/util/main-loop.c:516:11 #11 0x560629094a64 in qemu_main_loop /home/alxndr/Development/qemu/softmmu/vl.c:1676:9 #12 0x56062a749ab5 in main /home/alxndr/Development/qemu/softmmu/main.c:49:5 #13 0x7f0d5cd55e0a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26e0a) #14 0x5606288ba889 in _start (/home/alxndr/Development/qemu/build/i386-softmmu/qemu-system-i386+0x24d0889) 0x60d050e8 is located 56 bytes inside of 136-byte region [0x60d050b0,0x60d05138) freed by thread T0 here: #0 0x56062893250d in free (/home/alxndr/Development/qemu/build/i386-softmmu/qemu-system-i386+0x254850d) #1 0x560629827730 in virtio_gpu_reset /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:1160:9 #2 0x560628e81d34 in virtio_reset /home/alxndr/Development/qemu/hw/virtio/virtio.c:1999:9 #3 0x560629f08773 in virtio_pci_reset /home/alxndr/Development/qemu/hw/virtio/virtio-pci.c:1841:5 #4 0x560629043ab6 in memory_region_write_accessor /home/alxndr/Development/qemu/softmmu/memory.c:483:5 #5 0x560629043473 in access_with_adjusted_size /home/alxndr/Development/qemu/softmmu/memory.c:544:18 #6 0x560629042c99 in memory_region_dispatch_write /home/alxndr/Development/qemu/softmmu/memory.c #7 0x560628990a37 in flatview_write_continue /home/alxndr/Development/qemu/exec.c:3176:23 #8 0x56062899041a in address_space_write_cached_slow /home/alxndr/Development/qemu/exec.c:3789:12 #9 0x560628e6f9bb in vring_used_write /home/alxndr/Development/qemu/hw/virtio/virtio.c:347:5 #10 0x560628e6f9bb in virtqueue_split_fill /home/alxndr/Development/qemu/hw/virtio/virtio.c:788:5 #11 0x560628e6f9bb in virtqueue_fill /home/alxndr/Development/qemu/hw/virtio/virtio.c:852:9 #12 0x560628e7205e in virtqueue_push /home/alxndr/Development/qemu/hw/virtio/virtio.c:917:5 #13 0x560629814246 in virtio_gpu_ctrl_response /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:180:5 #14 0x56062981adc8 in virtio_gpu_ctrl_response_nodata /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:193:5 #15 0x56062981adc8 in virtio_gpu_simple_process_cmd /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:791:9 #16 0x5606298175f8 in virtio_gpu_process_cmdq /home/alxndr/Development/qemu/hw/display/virtio-gpu.c:820:9 #17 0x56062a8f1c96 in aio_bh_poll /home/alxndr/Development/qemu/util/async.c:164:13 #18 0x56062a887b9d in aio_dispatch /home/alxndr/Development/qemu/util/aio-posix.c:380:5 #19 0x56062a8f6b1c in aio_ctx_dispatch /home/alxndr/Development/qemu/util/async.c:306:5 #20 0x7f0d5e1cf9ed in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed)
[Bug 1886362] Re: Heap use-after-free in lduw_he_p through e1000e_write_to_rx_buffers
This should have been fixed by the qemu_receive_packet/network loopback patches from a few months ago -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1886362 Title: Heap use-after-free in lduw_he_p through e1000e_write_to_rx_buffers Status in QEMU: Incomplete Bug description: Hello, This reproducer causes a heap-use-after free. QEMU Built with --enable-sanitizers: cat << EOF | ./i386-softmmu/qemu-system-i386 -M q35,accel=qtest \ -qtest stdio -nographic -monitor none -serial none outl 0xcf8 0x80001010 outl 0xcfc 0xe102 outl 0xcf8 0x80001014 outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x800010a2 write 0xe102003b 0x1 0xff write 0xe1020103 0x1e 0xff055c5e5c30be4511d084ff write 0xe1020420 0x4 0x write 0xe1020424 0x4 0x write 0xe102042b 0x1 0xff write 0xe1020430 0x4 0x055c5e5c write 0x5c041 0x1 0x04 write 0x5c042 0x1 0x02 write 0x5c043 0x1 0xe1 write 0x5c048 0x1 0x8a write 0x5c04a 0x1 0x31 write 0x5c04b 0x1 0xff write 0xe1020403 0x1 0xff EOF The Output: ==22689==ERROR: AddressSanitizer: heap-use-after-free on address 0x62500026800e at pc 0x55b93bb18bfa bp 0x7fffdbe844f0 sp 0x7fffdbe83cb8 READ of size 2 at 0x62500026800e thread T0 #0 in __asan_memcpy (/build/i386-softmmu/qemu-system-i386+) #1 in lduw_he_p /include/qemu/bswap.h:332:5 #2 in ldn_he_p /include/qemu/bswap.h:550:1 #3 in flatview_write_continue /exec.c:3145:19 #4 in flatview_write /exec.c:3186:14 #5 in address_space_write /exec.c:3280:18 #6 in address_space_rw /exec.c:3290:16 #7 in dma_memory_rw_relaxed /include/sysemu/dma.h:87:18 #8 in dma_memory_rw /include/sysemu/dma.h:113:12 #9 in pci_dma_rw /include/hw/pci/pci.h:789:5 #10 in pci_dma_write /include/hw/pci/pci.h:802:12 #11 in e1000e_write_to_rx_buffers /hw/net/e1000e_core.c:1412:9 #12 in e1000e_write_packet_to_guest /hw/net/e1000e_core.c:1582:21 #13 in e1000e_receive_iov /hw/net/e1000e_core.c:1709:9 #14 in e1000e_nc_receive_iov /hw/net/e1000e.c:213:12 #15 in net_tx_pkt_sendv /hw/net/net_tx_pkt.c:544:9 #16 in net_tx_pkt_send /hw/net/net_tx_pkt.c:620:9 #17 in net_tx_pkt_send_loopback /hw/net/net_tx_pkt.c:633:11 #18 in e1000e_tx_pkt_send /hw/net/e1000e_core.c:664:16 #19 in e1000e_process_tx_desc /hw/net/e1000e_core.c:743:17 #20 in e1000e_start_xmit /hw/net/e1000e_core.c:934:9 #21 in e1000e_set_tctl /hw/net/e1000e_core.c:2431:9 #22 in e1000e_core_write /hw/net/e1000e_core.c:3265:9 #23 in e1000e_mmio_write /hw/net/e1000e.c:109:5 #24 in memory_region_write_accessor /memory.c:483:5 #25 in access_with_adjusted_size /memory.c:544:18 #26 in memory_region_dispatch_write /memory.c:1476:16 #27 in flatview_write_continue /exec.c:3146:23 #28 in flatview_write /exec.c:3186:14 #29 in address_space_write /exec.c:3280:18 #30 in qtest_process_command /qtest.c:567:9 #31 in qtest_process_inbuf /qtest.c:710:9 #32 in qtest_read /qtest.c:722:5 #33 in qemu_chr_be_write_impl /chardev/char.c:188:9 #34 in qemu_chr_be_write /chardev/char.c:200:9 #35 in fd_chr_read /chardev/char-fd.c:68:9 #36 in qio_channel_fd_source_dispatch /io/channel-watch.c:84:12 #37 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+) #38 in glib_pollfds_poll /util/main-loop.c:219:9 #39 in os_host_main_loop_wait /util/main-loop.c:242:5 #40 in main_loop_wait /util/main-loop.c:518:11 #41 in qemu_main_loop /softmmu/vl.c:1664:9 #42 in main /softmmu/main.c:52:5 #43 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+) #44 in _start (/build/i386-softmmu/qemu-system-i386+) 0x62500026800e is located 14 bytes inside of 138-byte region [0x625000268000,0x62500026808a) freed by thread T0 here: #0 in free (/build/i386-softmmu/qemu-system-i386+) #1 in qemu_vfree /util/oslib-posix.c:238:5 #2 in address_space_unmap /exec.c:3616:5 #3 in dma_memory_unmap /include/sysemu/dma.h:148:5 #4 in pci_dma_unmap /include/hw/pci/pci.h:839:5 #5 in net_tx_pkt_reset /hw/net/net_tx_pkt.c:453:9 #6 in e1000e_process_tx_desc /hw/net/e1000e_core.c:749:9 #7 in e1000e_start_xmit /hw/net/e1000e_core.c:934:9 #8 in e1000e_set_tctl /hw/net/e1000e_core.c:2431:9 #9 in e1000e_core_write /hw/net/e1000e_core.c:3265:9 #10 in e1000e_mmio_write /hw/net/e1000e.c:109:5 #11 in memory_region_write_accessor /memory.c:483:5 #12 in access_with_adjusted_size /memory.c:544:18 #13 in memory_region_dispatch_write /memory.c:1476:16 #14 in flatview_write_continue /exec.c:3146:23 #15 in flatview_write /exec.c:3186:14 #16 in address_space_write
Re: [PATCH v2 0/8] configure: Change to -std=gnu11
Patchew URL: https://patchew.org/QEMU/20210614233143.1221879-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: 20210614233143.1221879-1-richard.hender...@linaro.org Subject: [PATCH v2 0/8] configure: Change to -std=gnu11 === 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 * [new tag] patchew/20210614233143.1221879-1-richard.hender...@linaro.org -> patchew/20210614233143.1221879-1-richard.hender...@linaro.org Switched to a new branch 'test' 1a974ee configure: Remove probe for _Static_assert 8a8c552 qemu/compiler: Remove QEMU_GENERIC d52ca80 include/qemu/lockable: Use _Generic instead of QEMU_GENERIC d751cc8 util: Use unique type for QemuRecMutex in thread-posix.h aafe079 util: Pass file+line to qemu_rec_mutex_unlock_impl 069b4e3 util: Use real functions for thread-posix QemuRecMutex df9952e softfloat: Use _Generic instead of QEMU_GENERIC 067736a configure: Use -std=gnu11 === OUTPUT BEGIN === 1/8 Checking commit 067736abd82a (configure: Use -std=gnu11) 2/8 Checking commit df9952ebdce3 (softfloat: Use _Generic instead of QEMU_GENERIC) ERROR: spaces required around that '*' (ctx:WxO) #22: FILE: fpu/softfloat.c:689: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #22: FILE: fpu/softfloat.c:689: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #23: FILE: fpu/softfloat.c:690: + FloatParts128 *: parts128_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #23: FILE: fpu/softfloat.c:690: + FloatParts128 *: parts128_##NAME) ^ ERROR: spaces required around that '*' (ctx:WxO) #28: FILE: fpu/softfloat.c:693: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #28: FILE: fpu/softfloat.c:693: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #29: FILE: fpu/softfloat.c:694: + FloatParts128 *: parts128_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #29: FILE: fpu/softfloat.c:694: + FloatParts128 *: parts128_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #30: FILE: fpu/softfloat.c:695: + FloatParts256 *: parts256_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #30: FILE: fpu/softfloat.c:695: + FloatParts256 *: parts256_##NAME) ^ ERROR: spaces required around that '*' (ctx:WxO) #39: FILE: fpu/softfloat.c:897: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #39: FILE: fpu/softfloat.c:897: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #40: FILE: fpu/softfloat.c:898: + FloatParts128 *: frac128_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #40: FILE: fpu/softfloat.c:898: + FloatParts128 *: frac128_##NAME) ^ ERROR: spaces required around that '*' (ctx:WxO) #45: FILE: fpu/softfloat.c:901: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #45: FILE: fpu/softfloat.c:901: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #46: FILE: fpu/softfloat.c:902: + FloatParts128 *: frac128_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #46: FILE: fpu/softfloat.c:902: + FloatParts128 *: frac128_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #47: FILE: fpu/softfloat.c:903: + FloatParts256 *: frac256_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #47: FILE: fpu/softfloat.c:903: + FloatParts256 *: frac256_##NAME) ^ total: 20 errors, 0 warnings, 32 lines checked Patch 2/8 has style problems, please review. If any of these errors
[Bug 1879223] Re: Assertion failure in e1000e_write_rx_descr
OSS-Fuzz never saw it. It was probably fixed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1879223 Title: Assertion failure in e1000e_write_rx_descr Status in QEMU: Incomplete Bug description: Hello, While fuzzing, I found an input which triggers an assertion failure in e1000e_write_rx_descr: qemu-system-i386: /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1359: void e1000e_write_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, size_t, uint16_t (*)[4]): Assertion `ps_hdr_len == 0' failed. Aborted #3 0x7684d092 in __GI___assert_fail (assertion=0x583703e0 "ps_hdr_len == 0", file=0x58361080 "/home/alxndr/Development/qemu/hw/net/e1000e_core.c", line=0x54f, function=0x58370420 <__PRETTY_FUNCTION__.e1000e_write_rx_descr> "void e1000e_write_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, size_t, uint16_t (*)[4])") at assert.c:101 #4 0x57206a58 in e1000e_write_rx_descr (core=0x7fffee0dd4e0, desc=0x7fff8720 "", pkt=0x0, rss_info=0x7fff8c50, ps_hdr_len=0x2a, written=0x7fff87c0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1359 #5 0x571f8507 in e1000e_write_packet_to_guest (core=0x7fffee0dd4e0, pkt=0x6114b900, rxr=0x7fff8c30, rss_info=0x7fff8c50) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1607 #6 0x571f5670 in e1000e_receive_iov (core=0x7fffee0dd4e0, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1709 #7 0x571f1afc in e1000e_nc_receive_iov (nc=0x61407460, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:213 #8 0x571d5977 in net_tx_pkt_sendv (pkt=0x63128800, nc=0x61407460, iov=0x6194e780, iov_cnt=0x4) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:544 #9 0x571d50e4 in net_tx_pkt_send (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:620 #10 0x571d638f in net_tx_pkt_send_loopback (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:633 #11 0x5722b600 in e1000e_tx_pkt_send (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:664 #12 0x57229ca6 in e1000e_process_tx_desc (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, dp=0x7fff9440, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:743 #13 0x57228ea5 in e1000e_start_xmit (core=0x7fffee0dd4e0, txr=0x7fff9640) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:934 #14 0x5721c70f in e1000e_set_tdt (core=0x7fffee0dd4e0, index=0xe06, val=0xcb) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:2451 #15 0x571fa436 in e1000e_core_write (core=0x7fffee0dd4e0, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:3261 #16 0x571ed11c in e1000e_mmio_write (opaque=0x7fffee0da800, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:109 #17 0x565e78b2 in memory_region_write_accessor (mr=0x7fffee0dd110, addr=0x438, value=0x7fff9cb0, size=0x4, shift=0x0, mask=0x, attrs=...) at /home/alxndr/Development/qemu/memory.c:483 #18 0x565e7212 in access_with_adjusted_size (addr=0x438, value=0x7fff9cb0, size=0x1, access_size_min=0x4, access_size_max=0x4, access_fn=0x565e72e0 , mr=0x7fffee0dd110, attrs=...) at /home/alxndr/Development/qemu/memory.c:544 #19 0x565e5c31 in memory_region_dispatch_write (mr=0x7fffee0dd110, addr=0x438, data=0xcb, op=MO_8, attrs=...) at /home/alxndr/Development/qemu/memory.c:1476 #20 0x563f04b9 in flatview_write_continue (fv=0x60637880, addr=0xe1020438, attrs=..., ptr=0x6199ba80, len=0x1, addr1=0x438, l=0x1, mr=0x7fffee0dd110) at /home/alxndr/Development/qemu/exec.c:3137 #21 0x563df2dd in flatview_write (fv=0x60637880, addr=0xe1020077, attrs=..., buf=0x6199ba80, len=0x3c2) at /home/alxndr/Development/qemu/exec.c:3177 #22 0x563deded in address_space_write (as=0x608027a0, addr=0xe1020077, attrs=..., buf=0x6199ba80, len=0x3c2) at /home/alxndr/Development/qemu/exec.c:3268 I can reproduce this in qemu 5.0 using these qtest commands: cat << EOF | ./qemu-system-i386 \ -qtest stdio -nographic -monitor none -serial none \ -M pc-q35-5.0 outl 0xcf8 0x80001010 outl 0xcfc 0xe102 outl 0xcf8 0x80001014 outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x800010a2 write 0xe1025008 0x4 0xfbffa3fa write 0xed040c 0x3 0x080047 write 0xe1020077 0x3c2
[Bug 1879227] Re: Assertion failure in e1000e_write_lgcy_rx_descr
OSS-Fuzz never saw it. It was probably fixed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1879227 Title: Assertion failure in e1000e_write_lgcy_rx_descr Status in QEMU: Incomplete Bug description: Hello, While fuzzing, I found an input which triggers an assertion failure in e1000e_write_lgcy_rx_descr: qemu-system-i386: /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1283: void e1000e_write_lgcy_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, uint16_t): Assertion `!rss_info->enabled' failed. Aborted #3 0x7684d092 in __GI___assert_fail (assertion=0x583704c0 "!rss_info->enabled", file=0x58361080 "/home/alxndr/Development/qemu/hw/net/e1000e_core.c", line=0x503, function=0x58370500 <__PRETTY_FUNCTION__.e1000e_write_lgcy_rx_descr> "void e1000e_write_lgcy_rx_descr(E1000ECore *, uint8_t *, struct NetRxPkt *, const E1000E_RSSInfo *, uint16_t)") at assert.c:101 #4 0x57209937 in e1000e_write_lgcy_rx_descr (core=0x7fffee0dd4e0, desc=0x7fff8720 "}}\253?", pkt=0x6114b900, rss_info=0x7fff8c50, length=0xcb) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1283 #5 0x57206b0b in e1000e_write_rx_descr (core=0x7fffee0dd4e0, desc=0x7fff8720 "}}\253?", pkt=0x6114b900, rss_info=0x7fff8c50, ps_hdr_len=0x0, written=0x7fff87c0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1360 #6 0x571f8507 in e1000e_write_packet_to_guest (core=0x7fffee0dd4e0, pkt=0x6114b900, rxr=0x7fff8c30, rss_info=0x7fff8c50) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1607 #7 0x571f5670 in e1000e_receive_iov (core=0x7fffee0dd4e0, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:1709 #8 0x571f1afc in e1000e_nc_receive_iov (nc=0x61407460, iov=0x6194e780, iovcnt=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:213 #9 0x571d5977 in net_tx_pkt_sendv (pkt=0x63128800, nc=0x61407460, iov=0x6194e780, iov_cnt=0x4) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:544 #10 0x571d50e4 in net_tx_pkt_send (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:620 #11 0x571d638f in net_tx_pkt_send_loopback (pkt=0x63128800, nc=0x61407460) at /home/alxndr/Development/qemu/hw/net/net_tx_pkt.c:633 #12 0x5722b600 in e1000e_tx_pkt_send (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:664 #13 0x57229ca6 in e1000e_process_tx_desc (core=0x7fffee0dd4e0, tx=0x7fffee0fd748, dp=0x7fff9440, queue_index=0x0) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:743 #14 0x57228ea5 in e1000e_start_xmit (core=0x7fffee0dd4e0, txr=0x7fff9640) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:934 #15 0x5721c70f in e1000e_set_tdt (core=0x7fffee0dd4e0, index=0xe06, val=0xcb) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:2451 #16 0x571fa436 in e1000e_core_write (core=0x7fffee0dd4e0, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e_core.c:3261 #17 0x571ed11c in e1000e_mmio_write (opaque=0x7fffee0da800, addr=0x438, val=0xcb, size=0x4) at /home/alxndr/Development/qemu/hw/net/e1000e.c:109 #18 0x565e78b2 in memory_region_write_accessor (mr=0x7fffee0dd110, addr=0x438, value=0x7fff9cb0, size=0x4, shift=0x0, mask=0x, attrs=...) at /home/alxndr/Development/qemu/memory.c:483 #19 0x565e7212 in access_with_adjusted_size (addr=0x438, value=0x7fff9cb0, size=0x1, access_size_min=0x4, access_size_max=0x4, access_fn=0x565e72e0 , mr=0x7fffee0dd110, attrs=...) at /home/alxndr/Development/qemu/memory.c:544 #20 0x565e5c31 in memory_region_dispatch_write (mr=0x7fffee0dd110, addr=0x438, data=0xcb, op=MO_8, attrs=...) at /home/alxndr/Development/qemu/memory.c:1476 #21 0x563f04b9 in flatview_write_continue (fv=0x60637880, addr=0xe1020438, attrs=..., ptr=0x6199ba80, len=0x1, addr1=0x438, l=0x1, mr=0x7fffee0dd110) at /home/alxndr/Development/qemu/exec.c:3137 #22 0x563df2dd in flatview_write (fv=0x60637880, addr=0xe10200a8, attrs=..., buf=0x6199ba80, len=0x391) at /home/alxndr/Development/qemu/exec.c:3177 I can reproduce this in qemu 5.0 using these qtest commands: cat << EOF | ./qemu-system-i386 \ -qtest stdio -nographic -monitor none -serial none \ -M pc-q35-5.0 outl 0xcf8 0x80001010 outl 0xcfc 0xe102 outl 0xcf8 0x80001014 outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x800010a2 write 0xe1025008 0x4 0xfbffa3fa write 0xed040c 0x3 0x080047 write 0xe1020077 0x3c2
[Bug 1880355] Re: Length restrictions for fw_cfg_dma_transfer?
This no longer causes timeout/excessive CPU usage for me. Probably fixed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1880355 Title: Length restrictions for fw_cfg_dma_transfer? Status in QEMU: Incomplete Bug description: For me, this takes close to 3 minutes at 100% CPU: echo "outl 0x518 0x9596" | ./i386-softmmu/qemu-system-i386 -M q35 -m 32 -nographic -accel qtest -monitor none -serial none -qtest stdio #0 phys_page_find (d=0x60635d80, addr=136728041144404) at /exec.c:338 #1 address_space_lookup_region (d=0x60635d80, addr=136728041144404, resolve_subpage=true) at /exec.c:363 #2 address_space_translate_internal (d=0x60635d80, addr=136728041144404, xlat=0x7fff1fc0d070, plen=0x7fff1fc0d090, resolve_subpage=true) at /exec.c:382 #3 flatview_do_translate (fv=0x60635d20, addr=136728041144404, xlat=0x7fff1fc0d070, plen_out=0x7fff1fc0d090, page_mask_out=0x0, is_write=true, is_mmio=true, target_as=0x7fff1fc0ce10, attrs=...) pment/qemu/exec.c:520 #4 flatview_translate (fv=0x60635d20, addr=136728041144404, xlat=0x7fff1fc0d070, plen=0x7fff1fc0d090, is_write=true, attrs=...) at /exec.c:586 #5 flatview_write_continue (fv=0x60635d20, addr=136728041144404, attrs=..., ptr=0x7fff1fc0d660, len=172, addr1=136728041144400, l=172, mr=0x557fd54e77e0 ) pment/qemu/exec.c:3160 #6 flatview_write (fv=0x60635d20, addr=136728041144064, attrs=..., buf=0x7fff1fc0d660, len=512) at /exec.c:3177 #7 address_space_write (as=0x557fd54e7a00 , addr=136728041144064, attrs=..., buf=0x7fff1fc0d660, len=512) at /exec.c:3271 #8 dma_memory_set (as=0x557fd54e7a00 , addr=136728041144064, c=0 '\000', len=1378422272) at /dma-helpers.c:31 #9 fw_cfg_dma_transfer (s=0x61a01e80) at /hw/nvram/fw_cfg.c:400 #10 fw_cfg_dma_mem_write (opaque=0x61a01e80, addr=4, value=4294940309, size=4) at /hw/nvram/fw_cfg.c:467 #11 memory_region_write_accessor (mr=0x61a02200, addr=4, value=0x7fff1fc0e3d0, size=4, shift=0, mask=4294967295, attrs=...) at /memory.c:483 #12 access_with_adjusted_size (addr=4, value=0x7fff1fc0e3d0, size=4, access_size_min=1, access_size_max=8, access_fn=0x557fd2288c80 , mr=0x61a02200, attrs=...) pment/qemu/memory.c:539 #13 memory_region_dispatch_write (mr=0x61a02200, addr=4, data=4294940309, op=MO_32, attrs=...) at /memory.c:1476 #14 flatview_write_continue (fv=0x60635f00, addr=1304, attrs=..., ptr=0x7fff1fc0ec40, len=4, addr1=4, l=4, mr=0x61a02200) at /exec.c:3137 #15 flatview_write (fv=0x60635f00, addr=1304, attrs=..., buf=0x7fff1fc0ec40, len=4) at /exec.c:3177 #16 address_space_write (as=0x557fd54e7bc0 , addr=1304, attrs=..., buf=0x7fff1fc0ec40, len=4) at /exec.c:3271 It looks like fw_cfg_dma_transfer gets the address(136728041144064) and length(1378422272) for the read from the value provided as input 4294940309 (0x9695) which lands in pcbios. Should there be any limits on the length of guest-memory that fw_cfg should populate? Found by libfuzzer To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1880355/+subscriptions
[PATCH v2 7/8] qemu/compiler: Remove QEMU_GENERIC
All previous users now use C11 _Generic. Signed-off-by: Richard Henderson --- include/qemu/compiler.h | 40 1 file changed, 40 deletions(-) diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index 091c45248b..5766d61589 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -173,46 +173,6 @@ #define QEMU_ALWAYS_INLINE #endif -/* Implement C11 _Generic via GCC builtins. Example: - * - *QEMU_GENERIC(x, (float, sinf), (long double, sinl), sin) (x) - * - * The first argument is the discriminator. The last is the default value. - * The middle ones are tuples in "(type, expansion)" format. - */ - -/* First, find out the number of generic cases. */ -#define QEMU_GENERIC(x, ...) \ -QEMU_GENERIC_(typeof(x), __VA_ARGS__, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0) - -/* There will be extra arguments, but they are not used. */ -#define QEMU_GENERIC_(x, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, count, ...) \ -QEMU_GENERIC##count(x, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9) - -/* Two more helper macros, this time to extract items from a parenthesized - * list. - */ -#define QEMU_FIRST_(a, b) a -#define QEMU_SECOND_(a, b) b - -/* ... and a final one for the common part of the "recursion". */ -#define QEMU_GENERIC_IF(x, type_then, else_) \ -__builtin_choose_expr(__builtin_types_compatible_p(x, \ - QEMU_FIRST_ type_then), \ - QEMU_SECOND_ type_then, else_) - -/* CPP poor man's "recursion". */ -#define QEMU_GENERIC1(x, a0, ...) (a0) -#define QEMU_GENERIC2(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC1(x, __VA_ARGS__)) -#define QEMU_GENERIC3(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC2(x, __VA_ARGS__)) -#define QEMU_GENERIC4(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC3(x, __VA_ARGS__)) -#define QEMU_GENERIC5(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC4(x, __VA_ARGS__)) -#define QEMU_GENERIC6(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC5(x, __VA_ARGS__)) -#define QEMU_GENERIC7(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC6(x, __VA_ARGS__)) -#define QEMU_GENERIC8(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC7(x, __VA_ARGS__)) -#define QEMU_GENERIC9(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC8(x, __VA_ARGS__)) -#define QEMU_GENERIC10(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC9(x, __VA_ARGS__)) - /** * qemu_build_not_reached() * -- 2.25.1
[PATCH v2 8/8] configure: Remove probe for _Static_assert
_Static_assert is part of C11, which is now required.
Signed-off-by: Richard Henderson
---
configure | 18 --
include/qemu/compiler.h | 11 ---
2 files changed, 29 deletions(-)
diff --git a/configure b/configure
index 0489864667..debd50c085 100755
--- a/configure
+++ b/configure
@@ -5090,20 +5090,6 @@ if compile_prog "" "" ; then
have_sysmacros=yes
fi
-##
-# check for _Static_assert()
-
-have_static_assert=no
-cat > $TMPC << EOF
-_Static_assert(1, "success");
-int main(void) {
-return 0;
-}
-EOF
-if compile_prog "" "" ; then
-have_static_assert=yes
-fi
-
##
# check for utmpx.h, it is missing e.g. on OpenBSD
@@ -6035,10 +6021,6 @@ if test "$have_sysmacros" = "yes" ; then
echo "CONFIG_SYSMACROS=y" >> $config_host_mak
fi
-if test "$have_static_assert" = "yes" ; then
- echo "CONFIG_STATIC_ASSERT=y" >> $config_host_mak
-fi
-
if test "$have_utmpx" = "yes" ; then
echo "HAVE_UTMPX=y" >> $config_host_mak
fi
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 5766d61589..3baa5e3790 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -72,18 +72,7 @@
int:(x) ? -1 : 1; \
}
-/* QEMU_BUILD_BUG_MSG() emits the message given if _Static_assert is
- * supported; otherwise, it will be omitted from the compiler error
- * message (but as it remains present in the source code, it can still
- * be useful when debugging). */
-#if defined(CONFIG_STATIC_ASSERT)
#define QEMU_BUILD_BUG_MSG(x, msg) _Static_assert(!(x), msg)
-#elif defined(__COUNTER__)
-#define QEMU_BUILD_BUG_MSG(x, msg) typedef QEMU_BUILD_BUG_ON_STRUCT(x) \
-glue(qemu_build_bug_on__, __COUNTER__) __attribute__((unused))
-#else
-#define QEMU_BUILD_BUG_MSG(x, msg)
-#endif
#define QEMU_BUILD_BUG_ON(x) QEMU_BUILD_BUG_MSG(x, "not expecting: " #x)
--
2.25.1
[PATCH v2 4/8] util: Pass file+line to qemu_rec_mutex_unlock_impl
Create macros for file+line expansion in qemu_rec_mutex_unlock
like we have for qemu_mutex_unlock.
Signed-off-by: Richard Henderson
---
include/qemu/thread.h| 10 +-
util/qemu-thread-posix.c | 4 ++--
util/qemu-thread-win32.c | 2 +-
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 2c0d85f3bc..460568d67d 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -32,7 +32,7 @@ void qemu_rec_mutex_init(QemuRecMutex *mutex);
void qemu_rec_mutex_destroy(QemuRecMutex *mutex);
void qemu_rec_mutex_lock_impl(QemuRecMutex *mutex, const char *file, int line);
int qemu_rec_mutex_trylock_impl(QemuRecMutex *mutex, const char *file, int
line);
-void qemu_rec_mutex_unlock(QemuRecMutex *mutex);
+void qemu_rec_mutex_unlock_impl(QemuRecMutex *mutex, const char *file, int
line);
typedef void (*QemuMutexLockFunc)(QemuMutex *m, const char *f, int l);
typedef int (*QemuMutexTrylockFunc)(QemuMutex *m, const char *f, int l);
@@ -110,6 +110,9 @@ extern QemuCondTimedWaitFunc qemu_cond_timedwait_func;
#define qemu_mutex_unlock(mutex) \
qemu_mutex_unlock_impl(mutex, __FILE__, __LINE__)
+#define qemu_rec_mutex_unlock(mutex) \
+qemu_rec_mutex_unlock_impl(mutex, __FILE__, __LINE__)
+
static inline void (qemu_mutex_lock)(QemuMutex *mutex)
{
qemu_mutex_lock(mutex);
@@ -135,6 +138,11 @@ static inline int (qemu_rec_mutex_trylock)(QemuRecMutex
*mutex)
return qemu_rec_mutex_trylock(mutex);
}
+static inline void (qemu_rec_mutex_unlock)(QemuRecMutex *mutex)
+{
+qemu_rec_mutex_unlock(mutex);
+}
+
void qemu_cond_init(QemuCond *cond);
void qemu_cond_destroy(QemuCond *cond);
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 8e2b6653f5..d990826ed8 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -139,9 +139,9 @@ int qemu_rec_mutex_trylock_impl(QemuRecMutex *mutex, const
char *file, int line)
return qemu_mutex_trylock_impl(mutex, file, line);
}
-void qemu_rec_mutex_unlock(QemuRecMutex *mutex)
+void qemu_rec_mutex_unlock_impl(QemuRecMutex *mutex, const char *file, int
line)
{
-qemu_mutex_unlock(mutex);
+qemu_mutex_unlock_impl(mutex, file, line);
}
void qemu_cond_init(QemuCond *cond)
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index cb5aa2018c..52eb19f351 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -105,7 +105,7 @@ int qemu_rec_mutex_trylock_impl(QemuRecMutex *mutex, const
char *file, int line)
return !TryEnterCriticalSection(>lock);
}
-void qemu_rec_mutex_unlock(QemuRecMutex *mutex)
+void qemu_rec_mutex_unlock_impl(QemuRecMutex *mutex, const char *file, int
line)
{
assert(mutex->initialized);
LeaveCriticalSection(>lock);
--
2.25.1
[Bug 1878057] Re: null-ptr dereference in megasas_command_complete
Looks like OSS-Fuzz has a reproducer that still works: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29192#c3 I'll move this one over to gitlab -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1878057 Title: null-ptr dereference in megasas_command_complete Status in QEMU: Incomplete Bug description: Hello, While fuzzing, I found an input that triggers a null-pointer dereference in megasas_command_complete: ==14959==ERROR: AddressSanitizer: SEGV on unknown address 0x0003 (pc 0x55b1d11b4df1 bp 0x7ffeb55ca450 sp 0x7ffeb55ca1e0 T0) ==14959==The signal is caused by a WRITE memory access. ==14959==Hint: address points to the zero page. #0 0x55b1d11b4df1 in megasas_command_complete /home/alxndr/Development/qemu/hw/scsi/megasas.c:1877:40 #1 0x55b1d11759ec in scsi_req_complete /home/alxndr/Development/qemu/hw/scsi/scsi-bus.c:1430:5 #2 0x55b1d115c98f in scsi_aio_complete /home/alxndr/Development/qemu/hw/scsi/scsi-disk.c:216:5 #3 0x55b1d151c638 in blk_aio_complete /home/alxndr/Development/qemu/block/block-backend.c:1375:9 #4 0x55b1d151c638 in blk_aio_complete_bh /home/alxndr/Development/qemu/block/block-backend.c:1385:5 #5 0x55b1d16f3a5b in aio_bh_call /home/alxndr/Development/qemu/util/async.c:136:5 #6 0x55b1d16f3a5b in aio_bh_poll /home/alxndr/Development/qemu/util/async.c:164:13 #7 0x55b1d16fe43e in aio_dispatch /home/alxndr/Development/qemu/util/aio-posix.c:380:5 #8 0x55b1d16f54fa in aio_ctx_dispatch /home/alxndr/Development/qemu/util/async.c:306:5 #9 0x7f47937c89ed in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e9ed) #10 0x55b1d16fbef4 in glib_pollfds_poll /home/alxndr/Development/qemu/util/main-loop.c:219:9 #11 0x55b1d16fbef4 in os_host_main_loop_wait /home/alxndr/Development/qemu/util/main-loop.c:242:5 #12 0x55b1d16fbef4 in main_loop_wait /home/alxndr/Development/qemu/util/main-loop.c:518:11 #13 0x55b1d0cd16a6 in qemu_main_loop /home/alxndr/Development/qemu/softmmu/vl.c:1664:9 #14 0x55b1d1608dca in main /home/alxndr/Development/qemu/softmmu/main.c:49:5 #15 0x7f4792378e0a in __libc_start_main /build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16 #16 0x55b1d091d7b9 in _start (/home/alxndr/Development/qemu/build/i386-softmmu/qemu-system-i386+0x8f47b9) I can reproduce it in qemu 5.0 built with using: cat << EOF | ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0 -no-shutdown -M q35 -device megasas -device scsi-cd,drive=null0 -blockdev driver=null-co,read-zeroes=on,node-name=null0 -nographic -qtest stdio -monitor none -serial none outl 0xcf8 0x80001814 outl 0xcfc 0xc021 outl 0xcf8 0x80001818 outl 0xcf8 0x80001804 outw 0xcfc 0x7 outl 0xcf8 0x80001810 outl 0xcfc 0xe10c outl 0xcf8 0x8000f810 write 0x44b20 0x1 0x35 write 0x44b00 0x1 0x03 write 0xc021e10c0040 0x81 0x014b0400013100014b0400013800014b0400013f00014b0400014600014b0400014d00014b0400015400014b0400015b00014b0400016200014b0400016900014b040001714b0400017700014b0400017e00014b0400018500014b0400018c00014b04 EOF I also attached the trace to this launchpad report, in case the formatting is broken: qemu-system-i386 -qtest stdio -monitor none -serial none -M pc-q35-5.0 -no-shutdown -M q35 -device megasas -device scsi-cd,drive=null0 -blockdev driver=null-co,read-zeroes=on,node-name=null0 -nographic < attachment Please let me know if I can provide any further info. -Alex To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1878057/+subscriptions
[PATCH v2 6/8] include/qemu/lockable: Use _Generic instead of QEMU_GENERIC
This is both more and less complicated than our expansion
using __builtin_choose_expr and __builtin_types_compatible_p.
The expansion through QEMU_MAKE_LOCKABLE_ doesn't work because
we're not emumerating all of the types within the same _Generic,
which results in errors about unhandled cases. We must also
handle void* explicitly, so that the NULL constant can be used.
Signed-off-by: Richard Henderson
---
include/qemu/lockable.h | 90 +++--
1 file changed, 41 insertions(+), 49 deletions(-)
diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index b620023141..86db7cb04c 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -24,79 +24,71 @@ struct QemuLockable {
QemuLockUnlockFunc *unlock;
};
-/* This function gives an error if an invalid, non-NULL pointer type is passed
- * to QEMU_MAKE_LOCKABLE. For optimized builds, we can rely on dead-code
elimination
- * from the compiler, and give the errors already at link time.
- */
-#if defined(__OPTIMIZE__) && !defined(__SANITIZE_ADDRESS__)
-void unknown_lock_type(void *);
-#else
-static inline void unknown_lock_type(void *unused)
-{
-abort();
-}
-#endif
-
static inline __attribute__((__always_inline__)) QemuLockable *
qemu_make_lockable(void *x, QemuLockable *lockable)
{
-/* We cannot test this in a macro, otherwise we get compiler
+/*
+ * We cannot test this in a macro, otherwise we get compiler
* warnings like "the address of 'm' will always evaluate as 'true'".
*/
return x ? lockable : NULL;
}
-/* Auxiliary macros to simplify QEMU_MAKE_LOCABLE. */
-#define QEMU_LOCK_FUNC(x) ((QemuLockUnlockFunc *)\
-QEMU_GENERIC(x, \
- (QemuMutex *, qemu_mutex_lock), \
- (QemuRecMutex *, qemu_rec_mutex_lock), \
- (CoMutex *, qemu_co_mutex_lock),\
- (QemuSpin *, qemu_spin_lock), \
- unknown_lock_type))
+static inline __attribute__((__always_inline__)) QemuLockable *
+qemu_null_lockable(void *x)
+{
+if (x != NULL) {
+qemu_build_not_reached();
+}
+return NULL;
+}
-#define QEMU_UNLOCK_FUNC(x) ((QemuLockUnlockFunc *) \
-QEMU_GENERIC(x, \
- (QemuMutex *, qemu_mutex_unlock), \
- (QemuRecMutex *, qemu_rec_mutex_unlock), \
- (CoMutex *, qemu_co_mutex_unlock), \
- (QemuSpin *, qemu_spin_unlock), \
- unknown_lock_type))
-
-/* In C, compound literals have the lifetime of an automatic variable.
+/*
+ * In C, compound literals have the lifetime of an automatic variable.
* In C++ it would be different, but then C++ wouldn't need QemuLockable
* either...
*/
-#define QEMU_MAKE_LOCKABLE_(x) (&(QemuLockable) { \
-.object = (x), \
-.lock = QEMU_LOCK_FUNC(x), \
-.unlock = QEMU_UNLOCK_FUNC(x), \
+#define QML_OBJ_(x, name) (&(QemuLockable) {\
+.object = (x), \
+.lock = (QemuLockUnlockFunc *) qemu_ ## name ## _lock, \
+.unlock = (QemuLockUnlockFunc *) qemu_ ## name ## _unlock \
})
-/* QEMU_MAKE_LOCKABLE - Make a polymorphic QemuLockable
+/**
+ * QEMU_MAKE_LOCKABLE - Make a polymorphic QemuLockable
*
- * @x: a lock object (currently one of QemuMutex, QemuRecMutex, CoMutex,
QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
+ * CoMutex, QemuSpin).
*
* Returns a QemuLockable object that can be passed around
* to a function that can operate with locks of any kind, or
* NULL if @x is %NULL.
- */
-#define QEMU_MAKE_LOCKABLE(x)\
-QEMU_GENERIC(x, \
- (QemuLockable *, (x)), \
- qemu_make_lockable((x), QEMU_MAKE_LOCKABLE_(x)))
-
-/* QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
*
- * @x: a lock object (currently one of QemuMutex, QemuRecMutex, CoMutex,
QemuSpin).
+ * Note the special case for void *, so that we may pass "NULL".
+ */
+#define QEMU_MAKE_LOCKABLE(x) \
+_Generic((x), QemuLockable *: (x), \
+ void *: qemu_null_lockable(x), \
+ QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)),\
+ QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x, rec_mutex)), \
+ CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)), \
+ QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
+
+/**
+ * QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
+ *
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
+ * CoMutex, QemuSpin).
*
* Returns a QemuLockable
[PATCH v2 5/8] util: Use unique type for QemuRecMutex in thread-posix.h
We will shortly convert lockable.h to _Generic, and we cannot
have two compatible types in the same expansion. Wrap QemuMutex
in a struct, and unwrap in qemu-thread-posix.c.
Signed-off-by: Richard Henderson
---
include/qemu/thread-posix.h | 10 --
util/qemu-thread-posix.c| 12 ++--
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index cf8bc90468..b792e6ef37 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -4,8 +4,6 @@
#include
#include
-typedef QemuMutex QemuRecMutex;
-
struct QemuMutex {
pthread_mutex_t lock;
#ifdef CONFIG_DEBUG_MUTEX
@@ -15,6 +13,14 @@ struct QemuMutex {
bool initialized;
};
+/*
+ * QemuRecMutex cannot be a typedef of QemuMutex lest we have two
+ * compatible cases in _Generic. See qemu/lockable.h.
+ */
+typedef struct QemuRecMutex {
+QemuMutex m;
+} QemuRecMutex;
+
struct QemuCond {
pthread_cond_t cond;
bool initialized;
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index d990826ed8..fd9d714038 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -116,32 +116,32 @@ void qemu_rec_mutex_init(QemuRecMutex *mutex)
pthread_mutexattr_init();
pthread_mutexattr_settype(, PTHREAD_MUTEX_RECURSIVE);
-err = pthread_mutex_init(>lock, );
+err = pthread_mutex_init(>m.lock, );
pthread_mutexattr_destroy();
if (err) {
error_exit(err, __func__);
}
-mutex->initialized = true;
+mutex->m.initialized = true;
}
void qemu_rec_mutex_destroy(QemuRecMutex *mutex)
{
-qemu_mutex_destroy(mutex);
+qemu_mutex_destroy(>m);
}
void qemu_rec_mutex_lock_impl(QemuRecMutex *mutex, const char *file, int line)
{
-qemu_mutex_lock_impl(mutex, file, line);
+qemu_mutex_lock_impl(>m, file, line);
}
int qemu_rec_mutex_trylock_impl(QemuRecMutex *mutex, const char *file, int
line)
{
-return qemu_mutex_trylock_impl(mutex, file, line);
+return qemu_mutex_trylock_impl(>m, file, line);
}
void qemu_rec_mutex_unlock_impl(QemuRecMutex *mutex, const char *file, int
line)
{
-qemu_mutex_unlock_impl(mutex, file, line);
+qemu_mutex_unlock_impl(>m, file, line);
}
void qemu_cond_init(QemuCond *cond)
--
2.25.1
[PATCH v2 2/8] softfloat: Use _Generic instead of QEMU_GENERIC
Signed-off-by: Richard Henderson
---
fpu/softfloat.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 4d0160fe9c..6e769f990c 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -686,11 +686,13 @@ static float128 float128_pack_raw(const FloatParts128 *p)
#include "softfloat-specialize.c.inc"
#define PARTS_GENERIC_64_128(NAME, P) \
-QEMU_GENERIC(P, (FloatParts128 *, parts128_##NAME), parts64_##NAME)
+_Generic((P), FloatParts64 *: parts64_##NAME, \
+ FloatParts128 *: parts128_##NAME)
#define PARTS_GENERIC_64_128_256(NAME, P) \
-QEMU_GENERIC(P, (FloatParts256 *, parts256_##NAME), \
- (FloatParts128 *, parts128_##NAME), parts64_##NAME)
+_Generic((P), FloatParts64 *: parts64_##NAME, \
+ FloatParts128 *: parts128_##NAME, \
+ FloatParts256 *: parts256_##NAME)
#define parts_default_nan(P, S)PARTS_GENERIC_64_128(default_nan, P)(P, S)
#define parts_silence_nan(P, S)PARTS_GENERIC_64_128(silence_nan, P)(P, S)
@@ -892,11 +894,13 @@ static void parts128_log2(FloatParts128 *a, float_status
*s, const FloatFmt *f);
*/
#define FRAC_GENERIC_64_128(NAME, P) \
-QEMU_GENERIC(P, (FloatParts128 *, frac128_##NAME), frac64_##NAME)
+_Generic((P), FloatParts64 *: frac64_##NAME, \
+ FloatParts128 *: frac128_##NAME)
#define FRAC_GENERIC_64_128_256(NAME, P) \
-QEMU_GENERIC(P, (FloatParts256 *, frac256_##NAME), \
- (FloatParts128 *, frac128_##NAME), frac64_##NAME)
+_Generic((P), FloatParts64 *: frac64_##NAME, \
+ FloatParts128 *: frac128_##NAME, \
+ FloatParts256 *: frac256_##NAME)
static bool frac64_add(FloatParts64 *r, FloatParts64 *a, FloatParts64 *b)
{
--
2.25.1
[PATCH v2 0/8] configure: Change to -std=gnu11
Now that we assume gcc 7.5 as a minimum, we have the option of changing to a newer C standard. The two major new features that I think apply are _Generic and _Static_assert. While Paolo created a remarkably functional replacement for _Generic using builtins, the error messages that you get out of the keyword are *vastly* more intelligable, and the syntax is easier to read. While I'd like to prefer _Static_assert over QEMU_BUILD_BUG_ON going forward, and would like to convert existing uses, that is a much bigger job. Especially since the test condition is inverted. In the meantime, can drop the configure detection. Changes for v2: * Remove QEMU_LOCK_FUNC and QEMU_UNLOCK_FUNC as unused. r~ Richard Henderson (8): configure: Use -std=gnu11 softfloat: Use _Generic instead of QEMU_GENERIC util: Use real functions for thread-posix QemuRecMutex util: Pass file+line to qemu_rec_mutex_unlock_impl util: Use unique type for QemuRecMutex in thread-posix.h include/qemu/lockable: Use _Generic instead of QEMU_GENERIC qemu/compiler: Remove QEMU_GENERIC configure: Remove probe for _Static_assert configure | 22 + meson.build | 2 +- include/qemu/compiler.h | 51 - include/qemu/lockable.h | 90 + include/qemu/thread-posix.h | 14 +++--- include/qemu/thread-win32.h | 6 --- include/qemu/thread.h | 15 ++- fpu/softfloat.c | 16 --- util/qemu-thread-posix.c| 24 +- util/qemu-thread-win32.c| 2 +- 10 files changed, 98 insertions(+), 144 deletions(-) -- 2.25.1
[PATCH v2 3/8] util: Use real functions for thread-posix QemuRecMutex
Move the declarations from thread-win32.h into thread.h
and remove the macro redirection from thread-posix.h.
This will be required by following cleanups.
Signed-off-by: Richard Henderson
---
include/qemu/thread-posix.h | 4
include/qemu/thread-win32.h | 6 --
include/qemu/thread.h | 9 ++---
util/qemu-thread-posix.c| 20
4 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index c903525062..cf8bc90468 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -5,10 +5,6 @@
#include
typedef QemuMutex QemuRecMutex;
-#define qemu_rec_mutex_destroy qemu_mutex_destroy
-#define qemu_rec_mutex_lock_implqemu_mutex_lock_impl
-#define qemu_rec_mutex_trylock_impl qemu_mutex_trylock_impl
-#define qemu_rec_mutex_unlock qemu_mutex_unlock
struct QemuMutex {
pthread_mutex_t lock;
diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
index d0a1a9597e..d95af4498f 100644
--- a/include/qemu/thread-win32.h
+++ b/include/qemu/thread-win32.h
@@ -18,12 +18,6 @@ struct QemuRecMutex {
bool initialized;
};
-void qemu_rec_mutex_destroy(QemuRecMutex *mutex);
-void qemu_rec_mutex_lock_impl(QemuRecMutex *mutex, const char *file, int line);
-int qemu_rec_mutex_trylock_impl(QemuRecMutex *mutex, const char *file,
-int line);
-void qemu_rec_mutex_unlock(QemuRecMutex *mutex);
-
struct QemuCond {
CONDITION_VARIABLE var;
bool initialized;
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 5435763184..2c0d85f3bc 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -28,6 +28,12 @@ int qemu_mutex_trylock_impl(QemuMutex *mutex, const char
*file, const int line);
void qemu_mutex_lock_impl(QemuMutex *mutex, const char *file, const int line);
void qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file, const int
line);
+void qemu_rec_mutex_init(QemuRecMutex *mutex);
+void qemu_rec_mutex_destroy(QemuRecMutex *mutex);
+void qemu_rec_mutex_lock_impl(QemuRecMutex *mutex, const char *file, int line);
+int qemu_rec_mutex_trylock_impl(QemuRecMutex *mutex, const char *file, int
line);
+void qemu_rec_mutex_unlock(QemuRecMutex *mutex);
+
typedef void (*QemuMutexLockFunc)(QemuMutex *m, const char *f, int l);
typedef int (*QemuMutexTrylockFunc)(QemuMutex *m, const char *f, int l);
typedef void (*QemuRecMutexLockFunc)(QemuRecMutex *m, const char *f, int l);
@@ -129,9 +135,6 @@ static inline int (qemu_rec_mutex_trylock)(QemuRecMutex
*mutex)
return qemu_rec_mutex_trylock(mutex);
}
-/* Prototypes for other functions are in thread-posix.h/thread-win32.h. */
-void qemu_rec_mutex_init(QemuRecMutex *mutex);
-
void qemu_cond_init(QemuCond *cond);
void qemu_cond_destroy(QemuCond *cond);
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index dcff5e7c5d..8e2b6653f5 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -124,6 +124,26 @@ void qemu_rec_mutex_init(QemuRecMutex *mutex)
mutex->initialized = true;
}
+void qemu_rec_mutex_destroy(QemuRecMutex *mutex)
+{
+qemu_mutex_destroy(mutex);
+}
+
+void qemu_rec_mutex_lock_impl(QemuRecMutex *mutex, const char *file, int line)
+{
+qemu_mutex_lock_impl(mutex, file, line);
+}
+
+int qemu_rec_mutex_trylock_impl(QemuRecMutex *mutex, const char *file, int
line)
+{
+return qemu_mutex_trylock_impl(mutex, file, line);
+}
+
+void qemu_rec_mutex_unlock(QemuRecMutex *mutex)
+{
+qemu_mutex_unlock(mutex);
+}
+
void qemu_cond_init(QemuCond *cond)
{
int err;
--
2.25.1
[PATCH v2 1/8] configure: Use -std=gnu11
Now that the minimum gcc version is 7.5, we can use C11.
This will allow lots of cleanups to the code, currently
hidden behind macros in include/qemu/compiler.h.
Signed-off-by: Richard Henderson
---
configure | 4 ++--
meson.build | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 8dcb9965b2..0489864667 100755
--- a/configure
+++ b/configure
@@ -159,7 +159,7 @@ update_cxxflags() {
# options which some versions of GCC's C++ compiler complain about
# because they only make sense for C programs.
QEMU_CXXFLAGS="$QEMU_CXXFLAGS -D__STDC_LIMIT_MACROS
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS"
-CONFIGURE_CXXFLAGS=$(echo "$CONFIGURE_CFLAGS" | sed
s/-std=gnu99/-std=gnu++11/)
+CONFIGURE_CXXFLAGS=$(echo "$CONFIGURE_CFLAGS" | sed
s/-std=gnu11/-std=gnu++11/)
for arg in $QEMU_CFLAGS; do
case $arg in
-Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
@@ -538,7 +538,7 @@ QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls
$QEMU_CFLAGS"
QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
$QEMU_CFLAGS"
# Flags that are needed during configure but later taken care of by Meson
-CONFIGURE_CFLAGS="-std=gnu99 -Wall"
+CONFIGURE_CFLAGS="-std=gnu11 -Wall"
CONFIGURE_LDFLAGS=
diff --git a/meson.build b/meson.build
index a2311eda6e..d8a92666fb 100644
--- a/meson.build
+++ b/meson.build
@@ -1,5 +1,5 @@
project('qemu', ['c'], meson_version: '>=0.55.0',
-default_options: ['warning_level=1', 'c_std=gnu99', 'cpp_std=gnu++11',
'b_colorout=auto'] +
+default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11',
'b_colorout=auto'] +
(meson.version().version_compare('>=0.56.0') ? [
'b_staticpic=false' ] : []),
version: run_command('head', meson.source_root() /
'VERSION').stdout().strip())
--
2.25.1
[Bug 1878323] Re: Assertion-failure in usb_detach
OSS-Fuzz never found it, though we are fuzzing a slightly different ehci configuration there. I made a note of the arguments we should start fuzzing on OSS-Fuzz, but I think this is safe to close. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1878323 Title: Assertion-failure in usb_detach Status in QEMU: Incomplete Bug description: Hello, While fuzzing, I found an input that triggers an assertion-failure in usb_detach /home/alxndr/Development/qemu/hw/usb/core.c:69: void usb_detach(USBPort *): Assertion `dev->state != USB_STATE_NOTATTACHED' failed. #3 0x76866092 in __GI___assert_fail (assertion=0x57fd2040 "dev->state != USB_STATE_NOTATTACHED", file=0x57fd1ec0 "/home/alxndr/Development/qemu/hw/usb/core.c", line=0x45, function=0x57fd2000 <__PRETTY_FUNCTION__.usb_detach> "void usb_detach(USBPort *)") at assert.c:101 #4 0x5723f0ce in usb_detach (port=0x6212df30) at /home/alxndr/Development/qemu/hw/usb/core.c:69 #5 0x572a05a4 in ehci_reset (opaque=0x6212d9f0) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:863 #6 0x572bf941 in ehci_opreg_write (ptr=0x6212d9f0, addr=0x0, val=0xbebebebe, size=0x4) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:1032 #7 0x564938b5 in memory_region_write_accessor (mr=0x6212dcb0, addr=0x0, value=0x7fffaad0, size=0x4, shift=0x0, mask=0x, attrs=...) at /home/alxndr/Development/qemu/memory.c:483 #8 0x5649328a in access_with_adjusted_size (addr=0x0, value=0x7fffaad0, size=0x4, access_size_min=0x1, access_size_max=0x4, access_fn=0x56493360 , mr=0x6212dcb0, attrs=...) at /home/alxndr/Development/qemu/memory.c:544 #9 0x56491df6 in memory_region_dispatch_write (mr=0x6212dcb0, addr=0x0, data=0xbebebebe, op=MO_32, attrs=...) at /home/alxndr/Development/qemu/memory.c:1476 #10 0x562cbbf4 in flatview_write_continue (fv=0x6063e600, addr=0xe020, attrs=..., ptr=0x62500026, len=0xfe0, addr1=0x0, l=0x4, mr=0x6212dcb0) at /home/alxndr/Development/qemu/exec.c:3137 #11 0x562bbad9 in flatview_write (fv=0x6063e600, addr=0xe000, attrs=..., buf=0x62500026, len=0x1000) at /home/alxndr/Development/qemu/exec.c:3177 #12 0x562bb609 in address_space_write (as=0x6212d328, addr=0xe000, attrs=..., buf=0x62500026, len=0x1000) at /home/alxndr/Development/qemu/exec.c:3268 #13 0x562c06a6 in address_space_unmap (as=0x6212d328, buffer=0x62500026, len=0x1000, is_write=0x1, access_len=0x1000) at /home/alxndr/Development/qemu/exec.c:3592 #14 0x57257d73 in dma_memory_unmap (as=0x6212d328, buffer=0x62500026, len=0x1000, dir=DMA_DIRECTION_FROM_DEVICE, access_len=0x1000) at /home/alxndr/Development/qemu/include/sysemu/dma.h:145 #15 0x57257c57 in usb_packet_unmap (p=0x611484c0, sgl=0x61148548) at /home/alxndr/Development/qemu/hw/usb/libhw.c:65 #16 0x572a5953 in ehci_free_packet (p=0x61148480) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:536 #17 0x572a4ed4 in ehci_cancel_queue (q=0x60d04f10) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:584 #18 0x572a49ab in ehci_free_queue (q=0x60d04f10, warn=0x0) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:611 #19 0x572b102d in ehci_queues_rip_device (ehci=0x6212d9f0, dev=0x62301d00, async=0x1) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:674 #20 0x572af7a3 in ehci_detach (port=0x6212df78) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:733 #21 0x5723f15c in usb_detach (port=0x6212df78) at /home/alxndr/Development/qemu/hw/usb/core.c:70 #22 0x572a05a4 in ehci_reset (opaque=0x6212d9f0) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:863 #23 0x572bf941 in ehci_opreg_write (ptr=0x6212d9f0, addr=0x0, val=0xbebebebe, size=0x4) at /home/alxndr/Development/qemu/hw/usb/hcd-ehci.c:1032 #24 0x564938b5 in memory_region_write_accessor (mr=0x6212dcb0, addr=0x0, value=0x7fffc410, size=0x4, shift=0x0, mask=0x, attrs=...) at /home/alxndr/Development/qemu/memory.c:483 #25 0x5649328a in access_with_adjusted_size (addr=0x0, value=0x7fffc410, size=0x4, access_size_min=0x1, access_size_max=0x4, access_fn=0x56493360 , mr=0x6212dcb0, attrs=...) at /home/alxndr/Development/qemu/memory.c:544 #26 0x56491df6 in memory_region_dispatch_write (mr=0x6212dcb0, addr=0x0, data=0xbebebebe, op=MO_32, attrs=...) at /home/alxndr/Development/qemu/memory.c:1476 #27 0x562cbbf4 in flatview_write_continue (fv=0x6063e600, addr=0xe020, attrs=..., ptr=0x62500026, len=0xfe0, addr1=0x0, l=0x4, mr=0x6212dcb0) at /home/alxndr/Development/qemu/exec.c:3137 #28 0x562bbad9 in flatview_write
Re: [PATCH 00/11] vl: compound properties for machines
Patchew URL:
https://patchew.org/QEMU/20210610133538.608390-1-pbonz...@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210610133538.608390-1-pbonz...@redhat.com
Subject: [PATCH 00/11] vl: compound properties for machines
=== 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
Switched to a new branch 'test'
e17f5c6 machine: add smp compound property
cbf01d3 machine: reject -smp dies!=1 for non-PC machines
8636c7f machine: pass QAPI struct to mc->smp_parse
b648458 machine: add error propagation to mc->smp_parse
5dcfcb8 machine: move common smp_parse code to caller
087b4ce machine: move dies from X86MachineState to CpuTopology
c03d517 qemu-option: remove now-dead code
7f7fb81 vl: switch -M parsing to keyval
60f4b40 keyval: introduce keyval_parse_into
8c619a4 keyval: introduce keyval_merge
cef10f7 qom: export more functions for use with non-UserCreatable objects
=== OUTPUT BEGIN ===
1/11 Checking commit cef10f7fc760 (qom: export more functions for use with
non-UserCreatable objects)
2/11 Checking commit 8c619a4f71c0 (keyval: introduce keyval_merge)
ERROR: line over 90 characters
#45: FILE: tests/unit/test-keyval.c:756:
+QDict *combined =
keyval_parse("opt1=ABC,opt2.sub1=def,opt2.sub2=GHI,opt2.sub3=JKL,opt3=xyz",
WARNING: line over 80 characters
#114: FILE: util/keyval.c:314:
+static void keyval_do_merge(QDict *old, const QDict *new, GString *str, Error
**errp)
ERROR: line over 90 characters
#124: FILE: util/keyval.c:324:
+error_setg(errp, "Parameter '%s%s' used inconsistently",
str->str, ent->key);
total: 2 errors, 1 warnings, 127 lines checked
Patch 2/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/11 Checking commit 60f4b4022843 (keyval: introduce keyval_parse_into)
WARNING: line over 80 characters
#26: FILE: include/qemu/option.h:150:
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char
*implied_key,
WARNING: line over 80 characters
#48: FILE: util/keyval.c:486:
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char
*implied_key,
total: 0 errors, 2 warnings, 78 lines checked
Patch 3/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/11 Checking commit 7f7fb8157001 (vl: switch -M parsing to keyval)
WARNING: line over 80 characters
#206: FILE: softmmu/vl.c:1592:
+error_setg(_err, "No machine specified, and there is no
default");
WARNING: line over 80 characters
#219: FILE: softmmu/vl.c:1598:
+error_append_hint(_err, "Use -machine help to list supported
machines\n");
WARNING: line over 80 characters
#256: FILE: softmmu/vl.c:1639:
+error_setg(errp, "Conflict between '%s' and '%s'", ent->key,
new_key);
WARNING: line over 80 characters
#300: FILE: softmmu/vl.c:1663:
+object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru",
value,
WARNING: line over 80 characters
#307: FILE: softmmu/vl.c:1670:
+object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kvm-shadow-mem",
value,
WARNING: line over 80 characters
#314: FILE: softmmu/vl.c:1677:
+object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kernel-irqchip",
value,
WARNING: line over 80 characters
#316: FILE: softmmu/vl.c:1679:
+object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), "kernel-irqchip",
value,
ERROR: line over 90 characters
#338: FILE: softmmu/vl.c:1804:
+object_set_properties_from_keyval(OBJECT(current_machine), qdict, false,
_fatal);
ERROR: line over 90 characters
#350: FILE: softmmu/vl.c:1834:
+semihosting_arg_fallback(current_machine->kernel_filename,
current_machine->kernel_cmdline);
ERROR: code indent should never use tabs
#414: FILE: softmmu/vl.c:2128:
+^I * Cannot merge string-valued and type-safe dictionaries, so JSON$
ERROR: code indent should never use tabs
#415: FILE: softmmu/vl.c:2129:
+^I * is not accepted yet for -M.$
ERROR: code indent should never use tabs
#416: FILE: softmmu/vl.c:2130:
+^I */$
ERROR: line over 90 characters
#526: FILE: softmmu/vl.c:3248:
+keyval_parse_into(machine_opts_dict, optarg, "type",
, _fatal);
total: 6 errors, 7 warnings, 536 lines checked
Patch 4/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/11 Checking commit c03d51714eb0 (qemu-option: remove now-dead code)
6/11 Checking commit 087b4ce1fcfb (machine: move dies from X86MachineState to
CpuTopology)
7/11 Checking commit 5dcfcb88130d
Re: [PATCH v2 00/37] target/riscv: support packed extension v0.9.4
Patchew URL: https://patchew.org/QEMU/20210610075908.3305506-1-zhiwei_...@c-sky.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210610075908.3305506-1-zhiwei_...@c-sky.com Subject: [PATCH v2 00/37] target/riscv: support packed extension v0.9.4 === 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 Switched to a new branch 'test' 58bab55 target/riscv: configure and turn on packed extension from command line aaa9443 target/riscv: RV64 Only 32-bit Packing Instructions fd98368 target/riscv: RV64 Only Non-SIMD 32-bit Shift Instructions 13ee829 target/riscv: RV64 Only 32-bit Parallel Multiply & Add Instructions a45bf09 target/riscv: RV64 Only 32-bit Multiply & Add Instructions afa9d9f target/riscv: RV64 Only 32-bit Multiply Instructions 5e47cf9 target/riscv: RV64 Only SIMD Q15 saturating Multiply Instructions 0707fb2 target/riscv: RV64 Only SIMD 32-bit Miscellaneous Instructions 9103b42 target/riscv: RV64 Only SIMD 32-bit Shift Instructions aa8562e target/riscv: RV64 Only SIMD 32-bit Add/Subtract Instructions 4e3c751 target/riscv: Non-SIMD Miscellaneous Instructions 98463d7 target/riscv: 32-bit Computation Instructions a2b5fa4 target/riscv: Non-SIMD Q31 saturation ALU Instructions 5ac11aa target/riscv: Non-SIMD Q15 saturation ALU Instructions 8f8cc98 target/riscv: Signed 16-bit Multiply with 64-bit Add/Subtract Instructions 562fe16 target/riscv: 32-bit Multiply 64-bit Add/Subtract Instructions abd68e9 target/riscv: 64-bit Add/Subtract Instructions 1101a08 target/riscv: 8-bit Multiply with 32-bit Add Instructions cade413 target/riscv: Partial-SIMD Miscellaneous Instructions 868fc8a target/riscv: Signed 16-bit Multiply 64-bit Add/Subtract Instructions 55ea8d5 target/riscv: Signed 16-bit Multiply 32-bit Add/Subtract Instructions fc7375d target/riscv: Signed MSW 32x16 Multiply and Add Instructions 14d0690 target/riscv: Signed MSW 32x32 Multiply and Add Instructions 75852f9 target/riscv: 16-bit Packing Instructions 4c0f92a target/riscv: 8-bit Unpacking Instructions da3eb1d target/riscv: SIMD 8-bit Miscellaneous Instructions cda90fe target/riscv: SIMD 16-bit Miscellaneous Instructions 2c1cebb target/riscv: SIMD 8-bit Multiply Instructions ea6538c target/riscv: SIMD 16-bit Multiply Instructions e6d145d target/riscv: SIMD 8-bit Compare Instructions c7dc098 target/riscv: SIMD 16-bit Compare Instructions 98fdd40 target/riscv: SIMD 8-bit Shift Instructions 161cf36 target/riscv: SIMD 16-bit Shift Instructions 52b81ce target/riscv: 8-bit Addition & Subtraction Instruction 51a264e target/riscv: 16-bit Addition & Subtraction Instructions 9bfdab5 target/riscv: Make the vector helper functions public 8966803 target/riscv: implementation-defined constant parameters === OUTPUT BEGIN === 1/37 Checking commit 896680352f63 (target/riscv: implementation-defined constant parameters) 2/37 Checking commit 9bfdab58457e (target/riscv: Make the vector helper functions public) 3/37 Checking commit 51a264e82cf5 (target/riscv: 16-bit Addition & Subtraction Instructions) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #132: new file mode 100644 ERROR: space prohibited after that '*' (ctx:BxW) #172: FILE: target/riscv/insn_trans/trans_rvp.c.inc:36: + void (* vecop)(TCGv, TCGv, TCGv), ^ ERROR: space prohibited after that '*' (ctx:BxW) #173: FILE: target/riscv/insn_trans/trans_rvp.c.inc:37: + void (* op)(TCGv, TCGv, TCGv)) ^ ERROR: space prohibited after that '*' (ctx:BxW) #198: FILE: target/riscv/insn_trans/trans_rvp.c.inc:62: +r_ool(DisasContext *ctx, arg_r *a, void (* fn)(TCGv, TCGv_ptr, TCGv, TCGv)) ^ total: 3 errors, 1 warnings, 617 lines checked Patch 3/37 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/37 Checking commit 52b81cea79fe (target/riscv: 8-bit Addition & Subtraction Instruction) 5/37 Checking commit 161cf360f41e (target/riscv: SIMD 16-bit Shift Instructions) ERROR: space prohibited after that '*' (ctx:BxW) #138: FILE: target/riscv/insn_trans/trans_rvp.c.inc:144: + void (* fn)(TCGv, TCGv_ptr, TCGv, TCGv)) ^ ERROR: space prohibited after that '*' (ctx:BxW) #158: FILE: target/riscv/insn_trans/trans_rvp.c.inc:164: + void (* vecop)(TCGv, TCGv, target_long), ^ ERROR: space prohibited after that '*' (ctx:BxW) #159: FILE: target/riscv/insn_trans/trans_rvp.c.inc:165: + void (* op)(TCGv, TCGv_ptr, TCGv, TCGv)) ^ total: 3 errors, 0 warnings, 289 lines checked Patch 5/37 has style
Re: [RFC PATCH] migration/rdma: Fix out of order wrid
Patchew URL:
https://patchew.org/QEMU/20210610085831.19779-1-lizhij...@cn.fujitsu.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210610085831.19779-1-lizhij...@cn.fujitsu.com
Subject: [RFC PATCH] migration/rdma: Fix out of order wrid
=== 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
Switched to a new branch 'test'
2d73991 migration/rdma: Fix out of order wrid
=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#76: FILE: migration/rdma.c:1618:
+if (rdma->ooo_wrid >= RDMA_WRID_SEND_CONTROL && rdma->ooo_wrid ==
wrid_requested) {
ERROR: line over 90 characters
#91: FILE: migration/rdma.c:1633:
+error_report("more than one out of order wird(%ld, %ld)",
rdma->ooo_wrid, wr_id);
ERROR: Error messages should not contain newlines
#94: FILE: migration/rdma.c:1636:
+error_report("get out of order wird(%ld)\n", wr_id);
total: 3 errors, 0 warnings, 40 lines checked
Commit 2d739918ecc4 (migration/rdma: Fix out of order wrid) has style problems,
please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210610085831.19779-1-lizhij...@cn.fujitsu.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] Test comment for git-publish
Patchew URL: https://patchew.org/QEMU/20210611162033.437690-1-mcasc...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210611162033.437690-1-mcasc...@redhat.com Subject: [PATCH] Test comment for git-publish === 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 Switched to a new branch 'test' 65950dc Test comment for git-publish === OUTPUT BEGIN === ERROR: do not use C99 // comments #21: FILE: hw/rdma/vmw/pvrdma_main.c:430: +pvrdma_exec_cmd(dev); // this is a test comment ERROR: Missing Signed-off-by: line(s) total: 2 errors, 0 warnings, 8 lines checked Commit 65950dcea24f (Test comment for git-publish) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210611162033.437690-1-mcasc...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 0/8] virtio-gpu: Add a default synchronization mechanism for blobs
Patchew URL: https://patchew.org/QEMU/20210610224837.670192-1-vivek.kasire...@intel.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210610224837.670192-1-vivek.kasire...@intel.com Subject: [PATCH v2 0/8] virtio-gpu: Add a default synchronization mechanism for blobs === 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 Switched to a new branch 'test' d7ca345 virtio-gpu: Add gl_flushed callback 2a73d47 virtio-gpu: Add dmabuf helpers for synchronization 0d1be46 ui/gtk-egl: Wait for the draw signal for dmabuf blobs 3fd1b59 ui: Create sync objects only for blobs 13095b9 ui/gtk: Implement wait_dmabuf function e2c7ec7 ui: Add a helper to wait on a dmabuf sync object ffbfd82 ui/egl: Add egl helpers to help with synchronization 4612c6a ui/gtk: Create a common release_dmabuf helper === OUTPUT BEGIN === 1/8 Checking commit 4612c6a8e283 (ui/gtk: Create a common release_dmabuf helper) 2/8 Checking commit ffbfd82727d9 (ui/egl: Add egl helpers to help with synchronization) ERROR: code indent should never use tabs #63: FILE: ui/egl-helpers.c:88: +^I^I^I^IEGL_SYNC_NATIVE_FENCE_ANDROID, NULL);$ total: 1 errors, 0 warnings, 67 lines checked Patch 2/8 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/8 Checking commit e2c7ec781b0c (ui: Add a helper to wait on a dmabuf sync object) 4/8 Checking commit 13095b996f1a (ui/gtk: Implement wait_dmabuf function) 5/8 Checking commit 3fd1b5907cbd (ui: Create sync objects only for blobs) 6/8 Checking commit 0d1be4679e6f (ui/gtk-egl: Wait for the draw signal for dmabuf blobs) 7/8 Checking commit 2a73d47f2458 (virtio-gpu: Add dmabuf helpers for synchronization) 8/8 Checking commit d7ca345c6ce8 (virtio-gpu: Add gl_flushed callback) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210610224837.670192-1-vivek.kasire...@intel.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] net: Replace TAB indentations with spaces
Patchew URL: https://patchew.org/QEMU/20210614183849.20622-1-em...@aabouzied.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210614183849.20622-1-em...@aabouzied.com Subject: [PATCH] net: Replace TAB indentations with spaces === 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 Switched to a new branch 'test' 1cc4a36 net: Replace TAB indentations with spaces === OUTPUT BEGIN === ERROR: spaces prohibited around that ':' (ctx:WxW) #68: FILE: hw/net/can/ctu_can_fd_frame.h:53: +uint32_t dlc : 4; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #69: FILE: hw/net/can/ctu_can_fd_frame.h:54: +uint32_t reserved_4 : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #70: FILE: hw/net/can/ctu_can_fd_frame.h:55: +uint32_t rtr : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #71: FILE: hw/net/can/ctu_can_fd_frame.h:56: +uint32_t ide : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #72: FILE: hw/net/can/ctu_can_fd_frame.h:57: +uint32_t fdf : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #73: FILE: hw/net/can/ctu_can_fd_frame.h:58: +uint32_t reserved_8 : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #74: FILE: hw/net/can/ctu_can_fd_frame.h:59: +uint32_t brs : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #75: FILE: hw/net/can/ctu_can_fd_frame.h:60: +uint32_t esi_rsv : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #76: FILE: hw/net/can/ctu_can_fd_frame.h:61: +uint32_t rwcnt : 5; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #77: FILE: hw/net/can/ctu_can_fd_frame.h:62: +uint32_t reserved_31_16 : 16; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #89: FILE: hw/net/can/ctu_can_fd_frame.h:64: +uint32_t reserved_31_16 : 16; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #90: FILE: hw/net/can/ctu_can_fd_frame.h:65: +uint32_t rwcnt : 5; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #91: FILE: hw/net/can/ctu_can_fd_frame.h:66: +uint32_t esi_rsv : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #92: FILE: hw/net/can/ctu_can_fd_frame.h:67: +uint32_t brs : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #93: FILE: hw/net/can/ctu_can_fd_frame.h:68: +uint32_t reserved_8 : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #94: FILE: hw/net/can/ctu_can_fd_frame.h:69: +uint32_t fdf : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #95: FILE: hw/net/can/ctu_can_fd_frame.h:70: +uint32_t ide : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #96: FILE: hw/net/can/ctu_can_fd_frame.h:71: +uint32_t rtr : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #97: FILE: hw/net/can/ctu_can_fd_frame.h:72: +uint32_t reserved_4 : 1; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #98: FILE: hw/net/can/ctu_can_fd_frame.h:73: +uint32_t dlc : 4; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #149: FILE: hw/net/can/ctu_can_fd_frame.h:108: +uint32_t identifier_ext : 18; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #150: FILE: hw/net/can/ctu_can_fd_frame.h:109: +uint32_t identifier_base: 11; ^ ERROR: spaces prohibited around that ':' (ctx:WxW) #151: FILE: hw/net/can/ctu_can_fd_frame.h:110: +uint32_t reserved_31_29
Re: [PULL 00/12] Net patches
Patchew URL: https://patchew.org/QEMU/20210611060024.46763-1-jasow...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210611060024.46763-1-jasow...@redhat.com Subject: [PULL 00/12] Net patches === 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/20210614200940.2056770-1-phi...@redhat.com -> patchew/20210614200940.2056770-1-phi...@redhat.com * [new tag] patchew/20210614202842.581640-1-mathieu.poir...@linaro.org -> patchew/20210614202842.581640-1-mathieu.poir...@linaro.org Switched to a new branch 'test' === OUTPUT BEGIN === checkpatch.pl: no revisions returned for revlist 'base..' === OUTPUT END === Test command exited with code: 255 The full log is available at http://patchew.org/logs/20210611060024.46763-1-jasow...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 00/28] tcg: bswap improvements
Patchew URL:
https://patchew.org/QEMU/20210614083800.1166166-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: 20210614083800.1166166-1-richard.hender...@linaro.org
Subject: [PATCH 00/28] tcg: bswap improvements
=== 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
Switched to a new branch 'test'
fdfe6d3 tcg/riscv: Remove MO_BSWAP handling
fb9c8e1 tcg/aarch64: Unset TCG_TARGET_HAS_MEMORY_BSWAP
75c5607 tcg/arm: Unset TCG_TARGET_HAS_MEMORY_BSWAP
a57e516 target/mips: Fix gen_mxu_s32ldd_s32lddr
e4a99d6 target/sh4: Improve swap.b translation
e8b1814 target/i386: Improve bswap translation
129291d target/arm: Improve REVSH
ff11784 target/arm: Improve vector REV
f23fd8e target/arm: Improve REV32
a53eb56 tcg: Make use of bswap flags in tcg_gen_qemu_st_*
ef89c56 tcg: Make use of bswap flags in tcg_gen_qemu_ld_*
13c8b52 tcg: Add flags argument to tcg_gen_bswap16_*, tcg_gen_bswap32_i64
68e5471 tcg: Handle new bswap flags during optimize
17db675 tcg/tci: Support bswap flags
ff37d58 tcg/mips: Support bswap flags in tcg_out_bswap32
718bb99 tcg/mips: Support bswap flags in tcg_out_bswap16
784afc4 tcg/s390: Support bswap flags
2f03527 tcg/ppc: Use power10 byte-reverse instructions
d446243 tcg/ppc: Support bswap flags
7e02157 tcg/ppc: Split out tcg_out_bswap64
17813eb tcg/ppc: Split out tcg_out_bswap32
3316b19 tcg/ppc: Split out tcg_out_bswap16
b075ca8 tcg/ppc: Split out tcg_out_sari{32,64}
3b8cbf1 tcg/ppc: Split out tcg_out_ext{8,16,32}s
8ddb299 tcg/arm: Support bswap flags
22141b0 tcg/aarch64: Support bswap flags
a88e86e tcg/i386: Support bswap flags
7355a7f tcg: Add flags argument to bswap opcodes
=== OUTPUT BEGIN ===
1/28 Checking commit 7355a7f32661 (tcg: Add flags argument to bswap opcodes)
2/28 Checking commit a88e86e6484d (tcg/i386: Support bswap flags)
3/28 Checking commit 22141b0c1f1d (tcg/aarch64: Support bswap flags)
4/28 Checking commit 8ddb299c393e (tcg/arm: Support bswap flags)
5/28 Checking commit 3b8cbf12df69 (tcg/ppc: Split out tcg_out_ext{8,16,32}s)
6/28 Checking commit b075ca8b4504 (tcg/ppc: Split out tcg_out_sari{32,64})
7/28 Checking commit 3316b1902c92 (tcg/ppc: Split out tcg_out_bswap16)
8/28 Checking commit 17813ebc9f38 (tcg/ppc: Split out tcg_out_bswap32)
9/28 Checking commit 7e02157c16ed (tcg/ppc: Split out tcg_out_bswap64)
10/28 Checking commit d44624327aee (tcg/ppc: Support bswap flags)
11/28 Checking commit 2f03527066a7 (tcg/ppc: Use power10 byte-reverse
instructions)
12/28 Checking commit 784afc41c21e (tcg/s390: Support bswap flags)
13/28 Checking commit 718bb99a5724 (tcg/mips: Support bswap flags in
tcg_out_bswap16)
14/28 Checking commit ff37d58c7324 (tcg/mips: Support bswap flags in
tcg_out_bswap32)
15/28 Checking commit 17db67555db3 (tcg/tci: Support bswap flags)
16/28 Checking commit 68e5471a5b41 (tcg: Handle new bswap flags during optimize)
ERROR: spaces required around that ':' (ctx:VxE)
#40: FILE: tcg/optimize.c:1033:
+CASE_OP_32_64(bswap16):
^
ERROR: spaces required around that ':' (ctx:VxE)
#93: FILE: tcg/optimize.c:1188:
+CASE_OP_32_64(bswap16):
^
ERROR: spaces required around that ':' (ctx:VxE)
#94: FILE: tcg/optimize.c:1189:
+CASE_OP_32_64(bswap32):
^
total: 3 errors, 0 warnings, 82 lines checked
Patch 16/28 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/28 Checking commit 13c8b52b1b7a (tcg: Add flags argument to
tcg_gen_bswap16_*, tcg_gen_bswap32_i64)
ERROR: code indent should never use tabs
#163: FILE: target/sh4/translate.c:680:
+^Itcg_gen_bswap16_i32(low, low, TCG_BSWAP_IZ | TCG_BSWAP_OZ);$
total: 1 errors, 0 warnings, 296 lines checked
Patch 17/28 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/28 Checking commit ef89c566b6f3 (tcg: Make use of bswap flags in
tcg_gen_qemu_ld_*)
19/28 Checking commit a53eb569d43b (tcg: Make use of bswap flags in
tcg_gen_qemu_st_*)
20/28 Checking commit f23fd8e08da1 (target/arm: Improve REV32)
21/28 Checking commit ff11784030c8 (target/arm: Improve vector REV)
22/28 Checking commit 129291d55edb (target/arm: Improve REVSH)
23/28 Checking commit e8b18140cc85 (target/i386: Improve bswap translation)
24/28 Checking commit e4a99d6a7db1 (target/sh4: Improve swap.b translation)
ERROR: code indent should never use tabs
#26: FILE: target/sh4/translate.c:679:
+^Itcg_gen_bswap16_i32(low, REG(B7_4), 0);$
total: 1 errors, 0 warnings, 9 lines checked
Patch 24/28 has
Re: [PATCH] hw/nvme: be more careful when deasserting IRQs
Patchew URL:
https://patchew.org/QEMU/20210610114624.304681-1-jakub.jer...@kernkonzept.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210610114624.304681-1-jakub.jer...@kernkonzept.com
Subject: [PATCH] hw/nvme: be more careful when deasserting IRQs
=== 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
* [new tag] patchew/20210610132505.5827-1-peter.mayd...@linaro.org ->
patchew/20210610132505.5827-1-peter.mayd...@linaro.org
* [new tag]
patchew/20210614135429.56475-1-jakub.jer...@kernkonzept.com ->
patchew/20210614135429.56475-1-jakub.jer...@kernkonzept.com
Switched to a new branch 'test'
aacc285 hw/nvme: be more careful when deasserting IRQs
=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#34: FILE: hw/nvme/ctrl.c:485:
+if (q && q->vector == cq->vector && q->head != q->tail)
[...]
total: 1 errors, 0 warnings, 39 lines checked
Commit aacc285402f0 (hw/nvme: be more careful when deasserting IRQs) has style
problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210610114624.304681-1-jakub.jer...@kernkonzept.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 0/8] configure: Change to -std=gnu11
Patchew URL: https://patchew.org/QEMU/20210611233347.653129-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: 20210611233347.653129-1-richard.hender...@linaro.org Subject: [PATCH 0/8] configure: Change to -std=gnu11 === 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 * [new tag] patchew/20210610100802.5888-1-vsement...@virtuozzo.com -> patchew/20210610100802.5888-1-vsement...@virtuozzo.com * [new tag] patchew/20210610142549.33220-1-zhangjiachen.jay...@bytedance.com -> patchew/20210610142549.33220-1-zhangjiachen.jay...@bytedance.com Switched to a new branch 'test' e40a971 configure: Remove probe for _Static_assert 1d4e941 qemu/compiler: Remove QEMU_GENERIC c6f4654 include/qemu/lockable: Use _Generic instead of QEMU_GENERIC f158a02 util: Use unique type for QemuRecMutex in thread-posix.h a26bcbe util: Pass file+line to qemu_rec_mutex_unlock_impl e1ba627 util: Use real functions for thread-posix QemuRecMutex 096aebf softfloat: Use _Generic instead of QEMU_GENERIC 1781658 configure: Use -std=gnu11 === OUTPUT BEGIN === 1/8 Checking commit 178165898450 (configure: Use -std=gnu11) 2/8 Checking commit 096aebfff18d (softfloat: Use _Generic instead of QEMU_GENERIC) ERROR: spaces required around that '*' (ctx:WxO) #22: FILE: fpu/softfloat.c:689: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #22: FILE: fpu/softfloat.c:689: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #23: FILE: fpu/softfloat.c:690: + FloatParts128 *: parts128_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #23: FILE: fpu/softfloat.c:690: + FloatParts128 *: parts128_##NAME) ^ ERROR: spaces required around that '*' (ctx:WxO) #28: FILE: fpu/softfloat.c:693: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #28: FILE: fpu/softfloat.c:693: +_Generic((P), FloatParts64 *: parts64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #29: FILE: fpu/softfloat.c:694: + FloatParts128 *: parts128_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #29: FILE: fpu/softfloat.c:694: + FloatParts128 *: parts128_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #30: FILE: fpu/softfloat.c:695: + FloatParts256 *: parts256_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #30: FILE: fpu/softfloat.c:695: + FloatParts256 *: parts256_##NAME) ^ ERROR: spaces required around that '*' (ctx:WxO) #39: FILE: fpu/softfloat.c:897: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #39: FILE: fpu/softfloat.c:897: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #40: FILE: fpu/softfloat.c:898: + FloatParts128 *: frac128_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #40: FILE: fpu/softfloat.c:898: + FloatParts128 *: frac128_##NAME) ^ ERROR: spaces required around that '*' (ctx:WxO) #45: FILE: fpu/softfloat.c:901: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #45: FILE: fpu/softfloat.c:901: +_Generic((P), FloatParts64 *: frac64_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #46: FILE: fpu/softfloat.c:902: + FloatParts128 *: frac128_##NAME, \ ^ ERROR: spaces required around that ':' (ctx:OxW) #46: FILE: fpu/softfloat.c:902: + FloatParts128 *: frac128_##NAME, \ ^ ERROR: spaces required around that '*' (ctx:WxO) #47: FILE: fpu/softfloat.c:903: + FloatParts256 *: frac256_##NAME) ^ ERROR: spaces required around that ':' (ctx:OxW) #47: FILE: fpu/softfloat.c:903: + FloatParts256 *: frac256_##NAME)
Re: [RFC PATCH v2 2/2] target/ppc: make gdb able to translate priviledged addresses
On 6/14/21 12:16 PM, Bruno Larsen (billionai) wrote: This patch changes ppc_cpu_get_phys_page_debug so that it is now able to translate both, priviledged and real mode addresses independently of whether the CPU executing it has those permissions This was mentioned by Fabiano as something that would be very useful to help with debugging, but could possibly constitute a security issue if that debug function can be called in some way by prodution code. the solution was implemented such that it would be trivial to wrap it around ifdefs for building only with --enable-debug, for instance, but we are not sure this is the best approach, hence why it is an RFC. Suggested-by: Fabiano Rosas Signed-off-by: Bruno Larsen (billionai) --- target/ppc/mmu_helper.c | 23 +++ 1 file changed, 23 insertions(+) I think the first part is unnecessary. Either the cpu is in supervisor mode or it isn't, and gdb should use the correct address space. If you really want to force supervisor lookup from a guest that is paused in usermode, I suppose you could force MSR.PR=1 while you're performing the access and set it back afterward. I think the second part is actively wrong -- real-mode address lookup will (for the most part) always succeed. Moreover, the gdb user will have no idea that you've silently changed addressing methods. r~
Re: [PATCH v2] Test comment for git-publish
Patchew URL: https://patchew.org/QEMU/20210611164305.440633-1-mcasc...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210611164305.440633-1-mcasc...@redhat.com Subject: [PATCH v2] Test comment for git-publish === 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 Switched to a new branch 'test' bf784f4 Test comment for git-publish === OUTPUT BEGIN === ERROR: do not use C99 // comments #21: FILE: hw/rdma/vmw/pvrdma_main.c:430: +pvrdma_exec_cmd(dev); // this is a test comment ERROR: Missing Signed-off-by: line(s) total: 2 errors, 0 warnings, 8 lines checked Commit bf784f48ac5f (Test comment for git-publish) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210611164305.440633-1-mcasc...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 00/57] target/arm: First slice of MVE implementation
On 6/14/21 8:09 AM, Peter Maydell wrote:
- pass only ESIZE, not H, to macros in mve_helper.c
I've been thinking about the H* macros again while reading this.
While H##ESIZE is an improvement over passing in HN, I think we can do better and force
the adjustment to match the type -- completely avoiding bugs you've caught at least twice
during SVE review.
The form I'm playing with today is
#ifdef HOST_WORDS_BIGENDIAN
#define HTADJ(p) (7 >> __builtin_ctz(sizeof(*(p
#define HBADJ(p) (7 & (7 << __builtin_ctz(sizeof(*(p)
#else
#define HTADJ(p) 0
#define HBADJ(p) 0
#endif
/**
* HT: adjust Host addressing by Type
* @p: data pointer
* @i: array index
*
* Adjust p[i] for host-endian addressing of sub-quadword values.
*/
#define HT(p, i) ((p)[(i) ^ HADJ(p)])
/**
* HB: adjust Host addressing by Bype
* @p: data pointer
* @i: byte offset
*
* Adjust p[i / sizeof(*p)] for host-endian addressing
* of sub-quadword values. Unlike HT, @i is not an array
* index but a byte offset.
*/
#define HB(p, i) (*(__typeof(p))((uintptr_t)(p) + ((i) ^ H1ADJ(p
void bt(unsigned char *x, long i) { H(x, i) = 0; }
void ht(unsigned short *x, long i) { H(x, i) = 0; }
void wt(unsigned int *x, long i) { H(x, i) = 0; }
void qt(unsigned long *x, long i) { H(x, i) = 0; }
void bb(unsigned char *x, long i) { H1(x, i) = 0; }
void hb(unsigned short *x, long i) { H1(x, i) = 0; }
void wb(unsigned int *x, long i) { H1(x, i) = 0; }
void qb(unsigned long *x, long i) { H1(x, i) = 0; }
This gives us
#define DO_1OP(OP, TYPE, FN)\
void HELPER(mve_##OP)(CPUARMState *env, void *vd, void *vm) \
{ \
TYPE *d = vd, *m = vm; \
uint16_t mask = mve_element_mask(env); \
unsigned e; \
unsigned const esize = sizeof(TYPE);\
for (e = 0; e < 16 / esize; e++, mask >>= esize) { \
mergemask((d, e), FN(HT(m, e)]), mask); \
} \
mve_advance_vpt(env); \
}
Thoughts? Especially on the naming of the macros?
If the idea appeals, I'll do a pass over the existing code.
r~
Re: [PATCH] tcg/arm: Fix tcg_out_op function signature
Patchew URL: https://patchew.org/QEMU/20210610224450.23425-1-jzivi...@suse.de/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210610224450.23425-1-jzivi...@suse.de Subject: [PATCH] tcg/arm: Fix tcg_out_op function signature === 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 * [new tag] patchew/20210614114357.1146725-1-vinc...@bernat.ch -> patchew/20210614114357.1146725-1-vinc...@bernat.ch * [new tag] patchew/20210614144407.134243-1-mre...@redhat.com -> patchew/20210614144407.134243-1-mre...@redhat.com Switched to a new branch 'test' === OUTPUT BEGIN === checkpatch.pl: no revisions returned for revlist 'base..' === OUTPUT END === Test command exited with code: 255 The full log is available at http://patchew.org/logs/20210610224450.23425-1-jzivi...@suse.de/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 00/57] target/arm: First slice of MVE implementation
Patchew URL: https://patchew.org/QEMU/20210614151007.4545-1-peter.mayd...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210614151007.4545-1-peter.mayd...@linaro.org Subject: [PATCH v2 00/57] target/arm: First slice of MVE implementation === 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 * [new tag] patchew/20210614052623.1657103-1-f4...@amsat.org -> patchew/20210614052623.1657103-1-f4...@amsat.org Switched to a new branch 'test' 114fdf4 target/arm: Make VMOV scalar <-> gpreg beatwise for MVE 3aadcfc target/arm: Implement MVE VADDV 6adb32d target/arm: Implement MVE VHCADD 2e99c0a target/arm: Implement MVE VCADD a0493d8 target/arm: Implement MVE VADC, VSBC 977dce5 target/arm: Implement MVE VRHADD ce1ac42 target/arm: Implement MVE VQDMULL (vector) a5c3651 target/arm: Implement MVE VQDMLSDH and VQRDMLSDH 7a6e20d target/arm: Implement MVE VQDMLADH and VQRDMLADH 83b6df5 target/arm: Implmement MVE VRSHL 173b26f target/arm: Implement MVE VSHL insn b48b35a target/arm: Implement MVE VQRSHL b4e1e88 target/arm: Implement MVE VQSHL (vector) c516319 target/arm: Implement MVE VQADD, VQSUB (vector) bd980e5 target/arm: Implement MVE VQDMULH, VQRDMULH (vector) eaa2cb1 target/arm: Implement MVE VQDMULL scalar b6b168b target/arm: Implement MVE VQDMULH and VQRDMULH (scalar) e1dbfde target/arm: Implement MVE VQADD and VQSUB 23f072f target/arm: Implement MVE VPST 9fbabfc target/arm: Implement MVE VBRSR 3ca182e target/arm: Implement MVE VHADD, VHSUB (scalar) c0615d8 target/arm: Implement MVE VSUB, VMUL (scalar) 9646c71 target/arm: Implement MVE VADD (scalar) e091fe1 target/arm: Implement MVE VRMLALDAVH, VRMLSLDAVH 04aee7c include/qemu/int128.h: Add function to create Int128 from int64_t 19f5544 target/arm: Implement MVE VMLSLDAV 7c74a0d target/arm: Implement MVE VMLALDAV 2eed292 target/arm: Implement MVE VMULL 71e60ea target/arm: Implement MVE VHADD, VHSUB 3a11f89 target/arm: Implement MVE VABD 3a95638 target/arm: Implement MVE VMAX, VMIN d537c24 target/arm: Implement MVE VRMULH 35d0181 target/arm: Implement MVE VMULH d3d5dcb target/arm: Implement MVE VADD, VSUB, VMUL e1083b8 target/arm: Implement MVE VAND, VBIC, VORR, VORN, VEOR af20e6a target/arm: Implement MVE VDUP 44b9fa5 tcg: Make gen_dup_i32() public 6acf6cf target/arm: Implement MVE VNEG 201f1e3 target/arm: Implement MVE VABS ebce351 target/arm: Implement MVE VMVN (register) 2bf919a target/arm: Implement MVE VREV16, VREV32, VREV64 7b8105a0 bitops.h: Provide hswap32(), hswap64(), wswap64() swapping operations 111b6a7 target/arm: Implement MVE VCLS 8282e7f target/arm: Implement MVE VCLZ 900481d target/arm: Move expand_pred_b() data to translate.c 3c7f001 target/arm: Implement widening/narrowing MVE VLDR/VSTR insns 356b0df target/arm: Implement MVE VLDR/VSTR (non-widening forms) 5472a37 target/arm: Add framework for MVE decode dd6a572 target/arm: Implement MVE LETP insn d707539 target/arm: Implement MVE DLSTP 2fef29f target/arm: Implement MVE WLSTP insn 9f0aa8b target/arm: Implement MVE LCTP 4958c81 target/arm: Let vfp_access_check() handle late NOCP checks 605a2b4 target/arm: Add handling for PSR.ECI/ICI 82900b2 target/arm: Handle VPR semantics in existing code 9734264 target/arm: Enable FPSCR.QC bit for MVE a6d9eb0 target/arm: Provide and use H8 and H1_8 macros === OUTPUT BEGIN === 1/57 Checking commit a6d9eb077652 (target/arm: Provide and use H8 and H1_8 macros) 2/57 Checking commit 9734264e7ccd (target/arm: Enable FPSCR.QC bit for MVE) 3/57 Checking commit 82900b28d7cd (target/arm: Handle VPR semantics in existing code) 4/57 Checking commit 605a2b46fccc (target/arm: Add handling for PSR.ECI/ICI) 5/57 Checking commit 4958c8116393 (target/arm: Let vfp_access_check() handle late NOCP checks) 6/57 Checking commit 9f0aa8b6738e (target/arm: Implement MVE LCTP) 7/57 Checking commit 2fef29fbfced (target/arm: Implement MVE WLSTP insn) 8/57 Checking commit d707539d6307 (target/arm: Implement MVE DLSTP) 9/57 Checking commit dd6a5724a472 (target/arm: Implement MVE LETP insn) 10/57 Checking commit 5472a374b257 (target/arm: Add framework for MVE decode) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #42: new file mode 100644 total: 0 errors, 1 warnings, 77 lines checked Patch 10/57 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 11/57 Checking commit 356b0df2f8db (target/arm: Implement MVE VLDR/VSTR (non-widening forms)) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #29: new file mode 100644 WARNING: Block comments use a
Re: tb_flush() calls causing long Windows XP boot times
Patchew URL: https://patchew.org/QEMU/bcb8773b-fc54-4c25-9b60-92c263165...@gmail.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: bcb8773b-fc54-4c25-9b60-92c263165...@gmail.com Subject: tb_flush() calls causing long Windows XP boot times === 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 * [new tag] patchew/20210610213906.1313440-1-ebl...@redhat.com -> patchew/20210610213906.1313440-1-ebl...@redhat.com * [new tag] patchew/20210614083800.1166166-1-richard.hender...@linaro.org -> patchew/20210614083800.1166166-1-richard.hender...@linaro.org Switched to a new branch 'test' 0c48784 tb_flush() calls causing long Windows XP boot times === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 8 lines checked Commit 0c48784df7c4 (tb_flush() calls causing long Windows XP boot times) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/bcb8773b-fc54-4c25-9b60-92c263165...@gmail.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 0/4] Emulator fixes to enable running NetBSD/alpha
Patchew URL: https://patchew.org/QEMU/20210613211549.18094-1-thor...@me.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210613211549.18094-1-thor...@me.com
Subject: [PATCH 0/4] Emulator fixes to enable running NetBSD/alpha
=== 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
* [new tag] patchew/20210610001424.209158-1-jiang.w...@bytedance.com
-> patchew/20210610001424.209158-1-jiang.w...@bytedance.com
* [new tag] patchew/20210612085101.61304-1...@weilnetz.de ->
patchew/20210612085101.61304-1...@weilnetz.de
Switched to a new branch 'test'
ae5541d alpha: Provide console information to the PALcode at start-up.
ed14ccc alpha: Provide a PCI-ISA bridge device node for guest OS's that expect
it
4ab8e91 alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.
8e9fae5 mc146818rtc: Make PF independent of PIE
=== OUTPUT BEGIN ===
1/4 Checking commit 8e9fae55ef15 (mc146818rtc: Make PF independent of PIE)
2/4 Checking commit 4ab8e91bd35e (alpha: Set minimum PCI device ID to 1 to
match Clipper IRQ mappings.)
WARNING: Block comments use a leading /* on a separate line
#46: FILE: hw/alpha/dp264.c:79:
+/* Init the chipset. Because we're using CLIPPER IRQ mappings,
WARNING: Block comments use * on subsequent lines
#47: FILE: hw/alpha/dp264.c:80:
+/* Init the chipset. Because we're using CLIPPER IRQ mappings,
+ the minimum PCI device IdSel is 1. */
WARNING: Block comments use a trailing */ on a separate line
#47: FILE: hw/alpha/dp264.c:80:
+ the minimum PCI device IdSel is 1. */
total: 0 errors, 3 warnings, 37 lines checked
Patch 2/4 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/4 Checking commit ed14ccc6e1b5 (alpha: Provide a PCI-ISA bridge device node
for guest OS's that expect it)
WARNING: Block comments use a leading /* on a separate line
#29: FILE: hw/alpha/typhoon.c:922:
+/* Init the PCI-ISA bridge. Technically, PCI-based Alphas shipped
WARNING: Block comments use * on subsequent lines
#30: FILE: hw/alpha/typhoon.c:923:
+/* Init the PCI-ISA bridge. Technically, PCI-based Alphas shipped
+ with one of three different PCI-ISA bridges:
WARNING: Block comments use a trailing */ on a separate line
#49: FILE: hw/alpha/typhoon.c:942:
+ Tsunami/Typhoon systems. */
WARNING: Block comments use a leading /* on a separate line
#62: FILE: hw/alpha/typhoon.c:978:
+/* The following was copied from hw/isa/i82378.c and modified to provide
WARNING: Block comments use * on subsequent lines
#63: FILE: hw/alpha/typhoon.c:979:
+/* The following was copied from hw/isa/i82378.c and modified to provide
+ only the minimal PCI device node. */
WARNING: Block comments use a trailing */ on a separate line
#63: FILE: hw/alpha/typhoon.c:979:
+ only the minimal PCI device node. */
total: 0 errors, 6 warnings, 130 lines checked
Patch 3/4 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/4 Checking commit ae5541d5739e (alpha: Provide console information to the
PALcode at start-up.)
WARNING: Block comments use a leading /* on a separate line
#25: FILE: hw/alpha/dp264.c:75:
+/* arg0 -> memory size
WARNING: Block comments use * on subsequent lines
#26: FILE: hw/alpha/dp264.c:76:
+/* arg0 -> memory size
+ arg1 -> kernel entry point
WARNING: Block comments use a trailing */ on a separate line
#32: FILE: hw/alpha/dp264.c:82:
+ See init_hwrpb() in the PALcode. */
ERROR: suspect code indent for conditional statements (4, 6)
#37: FILE: hw/alpha/dp264.c:87:
+if (!machine->enable_graphics)
+ cpus[0]->env.trap_arg2 |= (1 << 6);
ERROR: braces {} are necessary for all arms of this statement
#37: FILE: hw/alpha/dp264.c:87:
+if (!machine->enable_graphics)
[...]
total: 2 errors, 3 warnings, 20 lines checked
Patch 4/4 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210613211549.18094-1-thor...@me.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PULL 0/9] migration queue
Patchew URL: https://patchew.org/QEMU/20210609144512.211746-1-dgilb...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210609144512.211746-1-dgilb...@redhat.com Subject: [PULL 0/9] migration queue === 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 * [new tag] patchew/20210609184955.1193081-1-phi...@redhat.com -> patchew/20210609184955.1193081-1-phi...@redhat.com * [new tag] patchew/20210611120427.49736-1-berra...@redhat.com -> patchew/20210611120427.49736-1-berra...@redhat.com * [new tag] patchew/20210613141222.548357-1-lukas.juen...@greensocs.com -> patchew/20210613141222.548357-1-lukas.juen...@greensocs.com Switched to a new branch 'test' === OUTPUT BEGIN === checkpatch.pl: no revisions returned for revlist 'base..' === OUTPUT END === Test command exited with code: 255 The full log is available at http://patchew.org/logs/20210609144512.211746-1-dgilb...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 55/57] target/arm: Implement MVE VHCADD
On 6/14/21 8:10 AM, Peter Maydell wrote: Implement the MVE VHCADD insn, which is similar to VCADD but performs a halving step. This one overlaps with VADC. Signed-off-by: Peter Maydell --- target/arm/helper-mve.h| 8 target/arm/mve.decode | 8 ++-- target/arm/mve_helper.c| 2 ++ target/arm/translate-mve.c | 4 +++- 4 files changed, 19 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 53/57] target/arm: Implement MVE VADC, VSBC
On 6/14/21 8:10 AM, Peter Maydell wrote: Implement the MVE VADC and VSBC insns. These perform an add-with-carry or subtract-with-carry of the 32-bit elements in each lane of the input vectors, where the carry-out of each add is the carry-in of the next. The initial carry input is either 1 or is from FPSCR.C; the carry out at the end is written back to FPSCR.C. Signed-off-by: Peter Maydell --- target/arm/helper-mve.h| 5 target/arm/mve.decode | 5 target/arm/mve_helper.c| 52 ++ target/arm/translate-mve.c | 37 +++ 4 files changed, 99 insertions(+) Much better. Reviewed-by: Richard Henderson r~
Re: [PATCH V2 0/2] vhost-vDPA: vq notification map support
Patchew URL: https://patchew.org/QEMU/20210602084106.43186-1-jasow...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210602084106.43186-1-jasow...@redhat.com Subject: [PATCH V2 0/2] vhost-vDPA: vq notification map support === 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 a4716fd..1ea06ab master -> master - [tag update] patchew/20210505103702.521457-1-berra...@redhat.com -> patchew/20210505103702.521457-1-berra...@redhat.com - [tag update] patchew/20210510114328.21835-1-da...@redhat.com -> patchew/20210510114328.21835-1-da...@redhat.com - [tag update] patchew/20210526170432.343588-1-phi...@redhat.com -> patchew/20210526170432.343588-1-phi...@redhat.com - [tag update] patchew/20210601141805.206582-1-pet...@redhat.com -> patchew/20210601141805.206582-1-pet...@redhat.com * [new tag] patchew/20210602084106.43186-1-jasow...@redhat.com -> patchew/20210602084106.43186-1-jasow...@redhat.com - [tag update] patchew/20210602191125.525742-1-josemartin...@gmail.com -> patchew/20210602191125.525742-1-josemartin...@gmail.com - [tag update] patchew/20210603171259.27962-1-peter.mayd...@linaro.org -> patchew/20210603171259.27962-1-peter.mayd...@linaro.org Switched to a new branch 'test' === OUTPUT BEGIN === checkpatch.pl: no revisions returned for revlist 'base..' === OUTPUT END === Test command exited with code: 255 The full log is available at http://patchew.org/logs/20210602084106.43186-1-jasow...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v3 5/5] tpm: Return QMP error when TPM is disabled in build
On 6/14/21 4:12 PM, Philippe Mathieu-Daudé wrote: G I forgot to commit: -- >8 -- diff --git a/MAINTAINERS b/MAINTAINERS index 7d9cd290426..636bf2f5365 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2707,7 +2707,6 @@ TPM M: Stefan Berger S: Maintained F: tpm.c -F: stubs/tpm.c F: hw/tpm/* F: include/hw/acpi/tpm.h F: include/sysemu/tpm* --- Who is going to upstream the series? I could add this part ...
Re: [PATCH v2 45/57] target/arm: Implement MVE VQSHL (vector)
On 6/14/21 8:09 AM, Peter Maydell wrote:
Implement the MVE VQSHL insn (encoding T4, which is the
vector-shift-by-vector version).
The DO_SQSHL_OP and DO_UQSHL_OP macros here are derived from
the neon_helper.c code for qshl_u{8,16,32} and qshl_s{8,16,32}.
Signed-off-by: Peter Maydell
---
Reviewed-by: Richard Henderson
r~
Re: [PATCH v2 35/57] target/arm: Implement MVE VADD (scalar)
On 6/14/21 8:09 AM, Peter Maydell wrote: Implement the scalar form of the MVE VADD insn. This takes the scalar operand from a general purpose register. Signed-off-by: Peter Maydell --- Reviewed-by: Richard Henderson r~
Re: [PATCH v2 34/57] target/arm: Implement MVE VRMLALDAVH, VRMLSLDAVH
On 6/14/21 8:09 AM, Peter Maydell wrote: Implement the MVE VRMLALDAVH and VRMLSLDAVH insns, which accumulate the results of a rounded multiply of pairs of elements into a 72-bit accumulator, returning the top 64 bits in a pair of general purpose registers. Signed-off-by: Peter Maydell --- target/arm/helper-mve.h| 8 target/arm/mve.decode | 7 +++ target/arm/mve_helper.c| 37 + target/arm/translate-mve.c | 24 4 files changed, 76 insertions(+) Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v2 2/2] target/ppc: make gdb able to translate priviledged addresses
"Bruno Larsen (billionai)" writes:
> This patch changes ppc_cpu_get_phys_page_debug so that it is now
> able to translate both, priviledged and real mode addresses
> independently of whether the CPU executing it has those permissions
>
> This was mentioned by Fabiano as something that would be very useful to
> help with debugging, but could possibly constitute a security issue if
> that debug function can be called in some way by prodution code.
Thinking a bit more about this, I think we just need to make sure that
this is not called during the regular operation of the virtual
machine. So not as much a security issue, more of a correctness one.
> the
> solution was implemented such that it would be trivial to wrap it around
> ifdefs for building only with --enable-debug, for instance, but we are
> not sure this is the best approach, hence why it is an RFC.
>
> Suggested-by: Fabiano Rosas
> Signed-off-by: Bruno Larsen (billionai)
> ---
> target/ppc/mmu_helper.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 9dcdf88597..41c727c690 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2947,6 +2947,29 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr
> addr)
>cpu_mmu_index(>env, true), false)) {
> return raddr & TARGET_PAGE_MASK;
> }
> +
> +/*
> + * This is a fallback, in case we're asking for priviledged memory to
> + * be printed, but the PCU is not executing in a priviledged manner.
> + *
> + * The code could be considered a security vulnerability if
> + * this function can be called in a scenario that does not involve
> + * debugging.
> + * Given the name and how useful using real addresses may be for
> + * actually debugging, however, we decided to include it anyway and
> + * discuss how to best avoid the possible security concerns.
> + * The current plan is that, if there is a chance this code is called in
> + * a production environment, we can surround it with ifdefs so that it
> + * is only compiled with --enable-debug
> + */
> +/* attempt to translate first with virtual addresses */
> +if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, , , , 1, false) ||
> +ppc_xlate(cpu, addr, MMU_INST_FETCH, , , , 1, false) ||
> +/* if didn't work, attempt to translate with real addresses */
> +ppc_xlate(cpu, addr, MMU_DATA_LOAD, , , , 3, false) ||
> +ppc_xlate(cpu, addr, MMU_INST_FETCH, , , , 3, false)) {
> +return raddr & TARGET_PAGE_MASK;
> +}
If this is only used during debug we could just give it the highest
mmu_idx, no need for a fallback.
Now, it might be possible that people use GDB to debug some aspect of
the MMU emulation, in which case it would be more useful to have the GDB
access fail just as the CPU would. But from my perspective it would be
better to have GDB access all of the guest memory without restriction.
> return -1;
> }
Re: [PATCH v2 1/2] target/ppc: fix address translation bug for radix mmus
"Bruno Larsen (billionai)" writes:
> +/*
> + * These correspond to the mmu_idx values computed in
> + * hreg_compute_hflags_value. See the tables therein
> + */
> +static inline bool mmuidx_pr(int idx) { return !(idx & 1); }
> +/*
> + * If we want to use these macros for hash-style MMUs, we need to
> + * add an if or another macro here.
> + */
Don't these work just fine for hash as well? Except for Booke.
> +static inline bool mmuidx_real(int idx) { return idx & 2; }
> +static inline bool mmuidx_hv(int idx) { return idx & 4; }
> +
[PATCH v2 5/5] MAINTAINERS: Add maintainer for vhost-user RNG implementation
This patch adds entry for the vhost-user-rng related files. Signed-off-by: Mathieu Poirier --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 36055f14c594..4fedca72c183 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1951,6 +1951,15 @@ F: include/sysemu/rng*.h F: backends/rng*.c F: tests/qtest/virtio-rng-test.c +vhost-user-rng +M: Mathieu Poirier +S: Supported +F: docs/tools/vhost-user-rng.rst +F: hw/virtio/vhost-user-rng.c +F: hw/virtio/vhost-user-rng-pci.c +F: include/hw/virtio/vhost-user-rng.h +F: tools/vhost-user-rng/* + virtio-crypto M: Gonglei S: Supported -- 2.25.1
[PATCH v2 3/5] vhost-user-rng: backend: Add RNG vhost-user daemon implementation
This patch provides the vhost-user backend implementation to work
in tandem with the vhost-user-rng implementation of the QEMU VMM.
It uses the vhost-user API so that other VMM can re-use the interface
without having to write the driver again.
Signed-off-by: Mathieu Poirier
---
tools/meson.build| 8 +
tools/vhost-user-rng/50-qemu-rng.json.in | 5 +
tools/vhost-user-rng/main.c | 403 +++
tools/vhost-user-rng/meson.build | 10 +
4 files changed, 426 insertions(+)
create mode 100644 tools/vhost-user-rng/50-qemu-rng.json.in
create mode 100644 tools/vhost-user-rng/main.c
create mode 100644 tools/vhost-user-rng/meson.build
diff --git a/tools/meson.build b/tools/meson.build
index 3e5a0abfa29f..66b0a11fbb45 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -24,3 +24,11 @@ endif
if have_virtiofsd
subdir('virtiofsd')
endif
+
+have_virtiorng = (have_system and
+have_tools and
+'CONFIG_LINUX' in config_host)
+
+if have_virtiorng
+ subdir('vhost-user-rng')
+endif
diff --git a/tools/vhost-user-rng/50-qemu-rng.json.in
b/tools/vhost-user-rng/50-qemu-rng.json.in
new file mode 100644
index ..9186c3c6fe1d
--- /dev/null
+++ b/tools/vhost-user-rng/50-qemu-rng.json.in
@@ -0,0 +1,5 @@
+{
+ "description": "QEMU vhost-user-rng",
+ "type": "bridge",
+ "binary": "@libexecdir@/vhost-user-rng"
+}
diff --git a/tools/vhost-user-rng/main.c b/tools/vhost-user-rng/main.c
new file mode 100644
index ..c3b8f6922757
--- /dev/null
+++ b/tools/vhost-user-rng/main.c
@@ -0,0 +1,403 @@
+/*
+ * VIRTIO RNG Emulation via vhost-user
+ *
+ * Copyright (c) 2021 Mathieu Poirier
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#define G_LOG_DOMAIN "vhost-user-rng"
+#define G_LOG_USE_STRUCTURED 1
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "qemu/cutils.h"
+#include "subprojects/libvhost-user/libvhost-user-glib.h"
+#include "subprojects/libvhost-user/libvhost-user.h"
+
+#ifndef container_of
+#define container_of(ptr, type, member) ({ \
+const typeof(((type *) 0)->member) * __mptr = (ptr); \
+(type *) ((char *) __mptr - offsetof(type, member)); })
+#endif
+
+typedef struct {
+VugDev dev;
+struct itimerspec ts;
+timer_t rate_limit_timer;
+pthread_mutex_t rng_mutex;
+pthread_cond_t rng_cond;
+int64_t quota_remaining;
+bool activate_timer;
+GMainLoop *loop;
+} VuRNG;
+
+static gboolean print_cap, verbose;
+static gchar *source_path, *socket_path;
+static gint source_fd, socket_fd = -1;
+
+/* Defaults tailored on virtio-rng.c */
+static uint32_t period_ms = 1 << 16;
+static uint64_t max_bytes = INT64_MAX;
+
+static void check_rate_limit(union sigval sv)
+{
+VuRNG *rng = sv.sival_ptr;
+bool wakeup = false;
+
+pthread_mutex_lock(>rng_mutex);
+/*
+ * The timer has expired and the guest has used all available
+ * entropy, which means function vu_rng_handle_request() is waiting
+ * on us. As such wake it up once we're done here.
+ */
+if (rng->quota_remaining == 0) {
+wakeup = true;
+}
+
+/*
+ * Reset the entropy available to the guest and tell function
+ * vu_rng_handle_requests() to start the timer before using it.
+ */
+rng->quota_remaining = max_bytes;
+rng->activate_timer = true;
+pthread_mutex_unlock(>rng_mutex);
+
+if (wakeup) {
+pthread_cond_signal(>rng_cond);
+}
+}
+
+static void setup_timer(VuRNG *rng)
+{
+struct sigevent sev;
+int ret;
+
+memset(>ts, 0, sizeof(struct itimerspec));
+rng->ts.it_value.tv_sec = period_ms / 1000;
+rng->ts.it_value.tv_nsec = (period_ms % 1000) * 100;
+
+/*
+ * Call function check_rate_limit() as if it was the start of
+ * a new thread when the timer expires.
+ */
+sev.sigev_notify = SIGEV_THREAD;
+sev.sigev_notify_function = check_rate_limit;
+sev.sigev_value.sival_ptr = rng;
+/* Needs to be NULL if defaults attributes are to be used. */
+sev.sigev_notify_attributes = NULL;
+ret = timer_create(CLOCK_MONOTONIC, , >rate_limit_timer);
+if (ret < 0) {
+fprintf(stderr, "timer_create() failed\n");
+}
+
+}
+
+
+/* Virtio helpers */
+static uint64_t rng_get_features(VuDev *dev)
+{
+if (verbose) {
+g_info("%s: replying", __func__);
+}
+return 0;
+}
+
+static void rng_set_features(VuDev *dev, uint64_t features)
+{
+if (verbose && features) {
+g_autoptr(GString) s = g_string_new("Requested un-handled feature");
+g_string_append_printf(s, " 0x%" PRIx64 "", features);
+g_info("%s: %s", __func__, s->str);
+}
+}
+
+static void vu_rng_handle_requests(VuDev *dev, int qidx)
+{
+VuRNG *rng = container_of(dev, VuRNG, dev.parent);
[PATCH v2 4/5] docs: Add documentation for vhost based RNG implementation
Add description and example for the vhost-user based RNG implementation. Tailored on Viresh Kumar's vhost-user-i2c documentation. Signed-off-by: Mathieu Poirier --- docs/tools/vhost-user-rng.rst | 74 +++ 1 file changed, 74 insertions(+) create mode 100644 docs/tools/vhost-user-rng.rst diff --git a/docs/tools/vhost-user-rng.rst b/docs/tools/vhost-user-rng.rst new file mode 100644 index ..7f69d7bb3c58 --- /dev/null +++ b/docs/tools/vhost-user-rng.rst @@ -0,0 +1,74 @@ +QEMU vhost-user-rng - RNG emulation backend +=== + +Synopsis + + +**vhost-user-rng** [*OPTIONS*] + +Description +--- + +This program is a vhost-user backend that emulates a VirtIO random number +generator (RNG). It uses the host's random number generator pool, +/dev/urandom by default but configurable at will, to satisfy requests from +guests. + +This program is designed to work with QEMU's ``-device +vhost-user-rng-pci`` but should work with any virtual machine monitor +(VMM) that supports vhost-user. See the Examples section below. + +Options +--- + +.. program:: vhost-user-rng + +.. option:: -h, --help + + Print help. + +.. option:: -v, --verbose + + Increase verbosity of output + +.. option:: -s, --socket-path=PATH + + Listen on vhost-user UNIX domain socket at PATH. Incompatible with --fd. + +.. option:: -f, --fd=FDNUM + + Accept connections from vhost-user UNIX domain socket file descriptor FDNUM. + The file descriptor must already be listening for connections. + Incompatible with --socket-path. + +.. option:: -p, --period + + Rate, in milliseconds, at which the RNG hardware can generate random data. + Used in conjunction with the --max-bytes option. + +.. option:: -m, --max-bytes + + In conjuction with the --period parameter, provides the maximum number of byte + per milliseconds a RNG device can generate. + +Examples + + +The daemon should be started first: + +:: + + host# vhost-user-rng --socket-path=rng.sock --period=1000 --max-bytes=4096 + +The QEMU invocation needs to create a chardev socket the device can +use to communicate as well as share the guests memory over a memfd. + +:: + + host# qemu-system \ + -chardev socket,path=$(PATH)/rng.sock,id=rng0 \ + -device vhost-user-rng-pci,chardev=rng0 \ + -m 4096 \ + -object memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on \ + -numa node,memdev=mem \ + ... -- 2.25.1
