Re: [PATCH v2 1/5] hw/display/virtio-gpu-virgl: virtio_gpu_gl -> virtio_gpu_virgl
On 2023/04/29 1:48, Gurchetan Singh wrote:
From: Gurchetan Singh
The virtio-gpu GL device has a heavy dependence on virgl.
Acknowledge this by naming functions accurately.
Signed-off-by: Gurchetan Singh
Reviewed-by: Philippe Mathieu-Daudé
---
v1:
- (Philippe) virtio_gpu_virglrenderer_reset -->
virtio_gpu_virgl_reset_renderer
v2:
- (Akihiko) Fix unnecessary line break
hw/display/virtio-gpu-gl.c | 26 +-
hw/display/virtio-gpu-virgl.c | 2 +-
include/hw/virtio/virtio-gpu.h | 2 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index e06be60dfb..8573043b85 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -25,9 +25,9 @@
#include
-static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
- struct virtio_gpu_scanout *s,
- uint32_t resource_id)
+static void virtio_gpu_virgl_update_cursor(VirtIOGPU *g,
+ struct virtio_gpu_scanout *s,
+ uint32_t resource_id)
Now the last two parameters are misaligned.
{
uint32_t width, height;
uint32_t pixels, *data;
@@ -48,14 +48,14 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
free(data);
}
-static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
+static void virtio_gpu_virgl_flushed(VirtIOGPUBase *b)
{
VirtIOGPU *g = VIRTIO_GPU(b);
virtio_gpu_process_cmdq(g);
}
-static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+static void virtio_gpu_virgl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOGPU *g = VIRTIO_GPU(vdev);
VirtIOGPUGL *gl = VIRTIO_GPU_GL(vdev);
@@ -71,7 +71,7 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
}
if (gl->renderer_reset) {
gl->renderer_reset = false;
-virtio_gpu_virgl_reset(g);
+virtio_gpu_virgl_reset_renderer(g);
}
cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
@@ -87,7 +87,7 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
virtio_gpu_virgl_fence_poll(g);
}
-static void virtio_gpu_gl_reset(VirtIODevice *vdev)
+static void virtio_gpu_virgl_reset(VirtIODevice *vdev)
{
VirtIOGPU *g = VIRTIO_GPU(vdev);
VirtIOGPUGL *gl = VIRTIO_GPU_GL(vdev);
@@ -104,7 +104,7 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
}
}
-static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
+static void virtio_gpu_virgl_device_realize(DeviceState *qdev, Error **errp)
{
VirtIOGPU *g = VIRTIO_GPU(qdev);
@@ -143,13 +143,13 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
VirtIOGPUBaseClass *vbc = VIRTIO_GPU_BASE_CLASS(klass);
VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
-vbc->gl_flushed = virtio_gpu_gl_flushed;
-vgc->handle_ctrl = virtio_gpu_gl_handle_ctrl;
+vbc->gl_flushed = virtio_gpu_virgl_flushed;
+vgc->handle_ctrl = virtio_gpu_virgl_handle_ctrl;
vgc->process_cmd = virtio_gpu_virgl_process_cmd;
-vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
+vgc->update_cursor_data = virtio_gpu_virgl_update_cursor;
-vdc->realize = virtio_gpu_gl_device_realize;
-vdc->reset = virtio_gpu_gl_reset;
+vdc->realize = virtio_gpu_virgl_device_realize;
+vdc->reset = virtio_gpu_virgl_reset;
device_class_set_props(dc, virtio_gpu_gl_properties);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 1c47603d40..ffe4ec7f3d 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -599,7 +599,7 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
}
}
-void virtio_gpu_virgl_reset(VirtIOGPU *g)
+void virtio_gpu_virgl_reset_renderer(VirtIOGPU *g)
{
virgl_renderer_reset();
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..21b0f55bc8 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -281,7 +281,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd);
void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
-void virtio_gpu_virgl_reset(VirtIOGPU *g);
+void virtio_gpu_virgl_reset_renderer(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
Re: [PATCH v3 1/2] igb: RX descriptors handling cleanup
On 2023/04/28 21:51, Tomasz Dzieciol/VIM Integration (NC)
/SRPOL/Engineer/Samsung Electronics wrote:
Please don't ignore comments in reviews, and if you have a question with them
or you don't agree with them, please write so in a reply. You don't have to
post a new version quickly so take time to address all problems pointed out.
I assumed that comments referred only to places pointed in the code and fixed
only those places. Sorry about that. I will keep in mind that your comments are
more general and fix all the places, where array is passed as parameter.
Please split up those changes into separate patches.
I will extract TCP ACK detection removal and IPv6 extensions traffic detection
to separate patches. Those will be small patches in comparison to the rest of
cleanup, however those are functional changes.
Do *not*:
- suffix struct name with _st. The convention is not common in QEMU code base,
or even e1000e and igb do not always use the suffixes.
- use _.
ok, I was looking at E1000E_RingInfo_st, which was added recently with IGB code in commit
3a977deebe6b9a10043182e922f6883924ef21f5 ("Intrdocue igb device emulation").
It's just copied from e1000e code. Check for e1000e_core.c for history
older than that commit.
Re: [PULL 00/21] Migration 20230428 patches
On 4/29/23 21:14, Lukas Straub wrote: On Sat, 29 Apr 2023 19:45:07 +0100 Richard Henderson wrote: On 4/28/23 20:11, Juan Quintela wrote: The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6: Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100) are available in the Git repository at: https://gitlab.com/juan.quintela/qemu.git tags/migration-20230428-pull-request for you to fetch changes up to 05ecac612ec6a4bdb358e68554b4406ac2c8e25a: migration: Initialize and cleanup decompression in migration.c (2023-04-28 20:54:53 +0200) Migration Pull request (20230429 vintage) Hi In this series: - compression code cleanup (lukas) nice, nice, nice. - drop useless parameters from migration_tls* (juan) - first part of remove QEMUFileHooks series (juan) Please apply. Juan Quintela (8): multifd: We already account for this packet on the multifd thread migration: Move ram_stats to its own file migration-stats.[ch] migration: Rename ram_counters to mig_stats migration: Rename RAMStats to MigrationAtomicStats migration/rdma: Split the zero page case from acct_update_position migration/rdma: Unfold last user of acct_update_position() migration: Drop unused parameter for migration_tls_get_creds() migration: Drop unused parameter for migration_tls_client_create() Lukas Straub (13): qtest/migration-test.c: Add tests with compress enabled qtest/migration-test.c: Add postcopy tests with compress enabled ram.c: Let the compress threads return a CompressResult enum ram.c: Dont change param->block in the compress thread ram.c: Reset result after sending queued data ram.c: Do not call save_page_header() from compress threads ram.c: Call update_compress_thread_counts from compress_send_queued_data ram.c: Remove last ram.c dependency from the core compress code ram.c: Move core compression code into its own file ram.c: Move core decompression code into its own file ram compress: Assert that the file buffer matches the result ram-compress.c: Make target independent migration: Initialize and cleanup decompression in migration.c There are a bunch of migration failures in CI: https://gitlab.com/qemu-project/qemu/-/jobs/4201998343#L375 https://gitlab.com/qemu-project/qemu/-/jobs/4201998342#L428 https://gitlab.com/qemu-project/qemu/-/jobs/4201998340#L459 https://gitlab.com/qemu-project/qemu/-/jobs/4201998336#L4883 r~ Hmm, it doesn't always fail. Any way to get the testlog from the failed jobs? What you can get from the links above is all I have. But they're consistent, and new. r~
Re: [PULL 00/17] Block patches
On 4/28/23 13:39, Stefan Hajnoczi wrote: The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6: Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100) are available in the Git repository at: https://gitlab.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to d3c760be786571d83d5cea01953e543df4d76f51: docs/zoned-storage:add zoned emulation use case (2023-04-28 08:34:07 -0400) Pull request This pull request contains Sam Li's virtio-blk zoned storage work. These patches were dropped from my previous block pull request due to CI failures. More CI build failures, e.g. https://gitlab.com/qemu-project/qemu/-/jobs/4202086013#L1720 https://gitlab.com/qemu-project/qemu/-/jobs/4202085995#L4088 r~
Re: [PATCH v6 2/3] hw/sd/sdhci: Support big endian SD host controller interfaces
On Sat, Apr 29, 2023 at 01:46:26PM -0700, Guenter Roeck wrote:
> Hi,
>
> On Tue, Nov 01, 2022 at 11:29:33PM +0100, Philippe Mathieu-Daudé wrote:
> > Some SDHCI IP can be synthetized in various endianness:
> > https://github.com/u-boot/u-boot/blob/v2021.04/doc/README.fsl-esdhc
> >
> > - CONFIG_SYS_FSL_ESDHC_BE
> >
> >ESDHC IP is in big-endian mode. Accessing ESDHC registers can be
> >determined by ESDHC IP's endian mode or processor's endian mode.
> >
> > Our current implementation is little-endian. In order to support
> > big endianness:
> >
> > - Rename current MemoryRegionOps as sdhci_mmio_le_ops ('le')
> > - Add an 'endianness' property to SDHCIState (default little endian)
> > - Set the 'io_ops' field in realize() after checking the property
> > - Add the sdhci_mmio_be_ops (big-endian) MemoryRegionOps.
> >
> > Signed-off-by: Philippe Mathieu-Daudé
>
> With this patch in place (in qemu v8.0), I can no longer boot linux
> from SD card with sabrelite, mcimx6ul-evk, and mcimx7d-sabre emulations.
> I get the following persistent errors.
>
> [ 12.210101] sdhci-esdhc-imx 2194000.mmc:
> esdhc_wait_for_card_clock_gate_off: card clock still not gate off in 100us!.
> [ 12.213222] sdhci-esdhc-imx 2194000.mmc:
> esdhc_wait_for_card_clock_gate_off: card clock still not gate off in 100us!.
> [ 12.215072] sdhci-esdhc-imx 2194000.mmc:
> esdhc_wait_for_card_clock_gate_off: card clock still not gate off in 100us!.
> [ 12.218766] sdhci-esdhc-imx 219.mmc:
> esdhc_wait_for_card_clock_gate_off: card clock still not gate off in 100us!.
> [ 12.220441] sdhci-esdhc-imx 219.mmc:
> esdhc_wait_for_card_clock_gate_off: card clock still not gate off in 100us!.
> [ 12.221542] sdhci-esdhc-imx 219.mmc:
> esdhc_wait_for_card_clock_gate_off: card clock still not gate off in 100us!.
> [ 12.241544] sdhci-esdhc-imx 219.mmc:
> esdhc_wait_for_card_clock_gate_off: card clock still not gate off in 100us!.
> [ 12.242608] sdhci-esdhc-imx 219.mmc: card clock still not stable in
> 100us!.
>
> The emulations start to work again after reverting this patch.
>
Cause explained below.
> Guenter
>
> > ---
> > hw/sd/sdhci-internal.h | 1 +
> > hw/sd/sdhci.c | 32 +---
> > include/hw/sd/sdhci.h | 1 +
> > 3 files changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> > index 964570f8e8..5f3765f12d 100644
> > --- a/hw/sd/sdhci-internal.h
> > +++ b/hw/sd/sdhci-internal.h
> > @@ -308,6 +308,7 @@ extern const VMStateDescription sdhci_vmstate;
> > #define SDHC_CAPAB_REG_DEFAULT 0x057834b4
> >
> > #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
> > +DEFINE_PROP_UINT8("endianness", _state, endianness,
> > DEVICE_LITTLE_ENDIAN), \
> > DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
> > DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
> > DEFINE_PROP_UINT8("vendor", _state, vendor, SDHCI_VENDOR_NONE), \
> > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> > index 22c758ad91..289baa879e 100644
> > --- a/hw/sd/sdhci.c
> > +++ b/hw/sd/sdhci.c
> > @@ -1329,7 +1329,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t
> > val, unsigned size)
> > value >> shift, value >> shift);
> > }
> >
> > -static const MemoryRegionOps sdhci_mmio_ops = {
> > +static const MemoryRegionOps sdhci_mmio_le_ops = {
> > .read = sdhci_read,
> > .write = sdhci_write,
> > .impl = {
> > @@ -1344,6 +1344,21 @@ static const MemoryRegionOps sdhci_mmio_ops = {
> > .endianness = DEVICE_LITTLE_ENDIAN,
> > };
> >
> > +static const MemoryRegionOps sdhci_mmio_be_ops = {
> > +.read = sdhci_read,
> > +.write = sdhci_write,
> > +.impl = {
> > +.min_access_size = 4,
> > +.max_access_size = 4,
> > +},
> > +.valid = {
> > +.min_access_size = 1,
> > +.max_access_size = 4,
> > +.unaligned = false
> > +},
> > +.endianness = DEVICE_BIG_ENDIAN,
> > +};
> > +
> > static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
> > {
> > ERRP_GUARD();
> > @@ -1371,8 +1386,6 @@ void sdhci_initfn(SDHCIState *s)
> >
> > s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> > sdhci_raise_insertion_irq, s);
> > s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> > sdhci_data_transfer, s);
> > -
> > -s->io_ops = &sdhci_mmio_ops;
The reason for initializing io_ops here is that the same driver also
supports imx_usdhc. That function also sets io_ops to usdhc specific
io ops. This is now overwritten by ...
> > }
> >
> > void sdhci_uninitfn(SDHCIState *s)
> > @@ -1388,10 +1401,23 @@ void sdhci_common_realize(SDHCIState *s, Error
> > **errp)
> > {
> > ERRP_GUARD();
> >
> > +switch (s->endianness) {
> > +case DEVICE_LITTLE_ENDIAN:
> > +s->io_ops = &sdhci_mmio_le_ops;
> > +break;
> > +case DEVICE_BIG_ENDIAN:
> > +
Re: [PATCH v6 2/3] hw/sd/sdhci: Support big endian SD host controller interfaces
Hi,
On Tue, Nov 01, 2022 at 11:29:33PM +0100, Philippe Mathieu-Daudé wrote:
> Some SDHCI IP can be synthetized in various endianness:
> https://github.com/u-boot/u-boot/blob/v2021.04/doc/README.fsl-esdhc
>
> - CONFIG_SYS_FSL_ESDHC_BE
>
>ESDHC IP is in big-endian mode. Accessing ESDHC registers can be
>determined by ESDHC IP's endian mode or processor's endian mode.
>
> Our current implementation is little-endian. In order to support
> big endianness:
>
> - Rename current MemoryRegionOps as sdhci_mmio_le_ops ('le')
> - Add an 'endianness' property to SDHCIState (default little endian)
> - Set the 'io_ops' field in realize() after checking the property
> - Add the sdhci_mmio_be_ops (big-endian) MemoryRegionOps.
>
> Signed-off-by: Philippe Mathieu-Daudé
With this patch in place (in qemu v8.0), I can no longer boot linux
from SD card with sabrelite, mcimx6ul-evk, and mcimx7d-sabre emulations.
I get the following persistent errors.
[ 12.210101] sdhci-esdhc-imx 2194000.mmc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
[ 12.213222] sdhci-esdhc-imx 2194000.mmc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
[ 12.215072] sdhci-esdhc-imx 2194000.mmc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
[ 12.218766] sdhci-esdhc-imx 219.mmc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
[ 12.220441] sdhci-esdhc-imx 219.mmc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
[ 12.221542] sdhci-esdhc-imx 219.mmc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
[ 12.241544] sdhci-esdhc-imx 219.mmc: esdhc_wait_for_card_clock_gate_off:
card clock still not gate off in 100us!.
[ 12.242608] sdhci-esdhc-imx 219.mmc: card clock still not stable in
100us!.
The emulations start to work again after reverting this patch.
Guenter
> ---
> hw/sd/sdhci-internal.h | 1 +
> hw/sd/sdhci.c | 32 +---
> include/hw/sd/sdhci.h | 1 +
> 3 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> index 964570f8e8..5f3765f12d 100644
> --- a/hw/sd/sdhci-internal.h
> +++ b/hw/sd/sdhci-internal.h
> @@ -308,6 +308,7 @@ extern const VMStateDescription sdhci_vmstate;
> #define SDHC_CAPAB_REG_DEFAULT 0x057834b4
>
> #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
> +DEFINE_PROP_UINT8("endianness", _state, endianness,
> DEVICE_LITTLE_ENDIAN), \
> DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
> DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
> DEFINE_PROP_UINT8("vendor", _state, vendor, SDHCI_VENDOR_NONE), \
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 22c758ad91..289baa879e 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1329,7 +1329,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val,
> unsigned size)
> value >> shift, value >> shift);
> }
>
> -static const MemoryRegionOps sdhci_mmio_ops = {
> +static const MemoryRegionOps sdhci_mmio_le_ops = {
> .read = sdhci_read,
> .write = sdhci_write,
> .impl = {
> @@ -1344,6 +1344,21 @@ static const MemoryRegionOps sdhci_mmio_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> +static const MemoryRegionOps sdhci_mmio_be_ops = {
> +.read = sdhci_read,
> +.write = sdhci_write,
> +.impl = {
> +.min_access_size = 4,
> +.max_access_size = 4,
> +},
> +.valid = {
> +.min_access_size = 1,
> +.max_access_size = 4,
> +.unaligned = false
> +},
> +.endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
> {
> ERRP_GUARD();
> @@ -1371,8 +1386,6 @@ void sdhci_initfn(SDHCIState *s)
>
> s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> sdhci_raise_insertion_irq, s);
> s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> sdhci_data_transfer, s);
> -
> -s->io_ops = &sdhci_mmio_ops;
> }
>
> void sdhci_uninitfn(SDHCIState *s)
> @@ -1388,10 +1401,23 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
> {
> ERRP_GUARD();
>
> +switch (s->endianness) {
> +case DEVICE_LITTLE_ENDIAN:
> +s->io_ops = &sdhci_mmio_le_ops;
> +break;
> +case DEVICE_BIG_ENDIAN:
> +s->io_ops = &sdhci_mmio_be_ops;
> +break;
> +default:
> +error_setg(errp, "Incorrect endianness");
> +return;
> +}
> +
> sdhci_init_readonly_registers(s, errp);
> if (*errp) {
> return;
> }
> +
> s->buf_maxsz = sdhci_get_fifolen(s);
> s->fifo_buffer = g_malloc0(s->buf_maxsz);
>
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index 01a64c5442..a989fca3b2 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -96,
Re: [PULL 00/21] Migration 20230428 patches
On Sat, 29 Apr 2023 19:45:07 +0100 Richard Henderson wrote: > On 4/28/23 20:11, Juan Quintela wrote: > > The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6: > > > >Merge tag 'migration-20230427-pull-request' of > > https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 > > +0100) > > > > are available in the Git repository at: > > > >https://gitlab.com/juan.quintela/qemu.git > > tags/migration-20230428-pull-request > > > > for you to fetch changes up to 05ecac612ec6a4bdb358e68554b4406ac2c8e25a: > > > >migration: Initialize and cleanup decompression in migration.c > > (2023-04-28 20:54:53 +0200) > > > > > > Migration Pull request (20230429 vintage) > > > > Hi > > > > In this series: > > - compression code cleanup (lukas) > >nice, nice, nice. > > - drop useless parameters from migration_tls* (juan) > > - first part of remove QEMUFileHooks series (juan) > > > > Please apply. > > > > > > > > Juan Quintela (8): > >multifd: We already account for this packet on the multifd thread > >migration: Move ram_stats to its own file migration-stats.[ch] > >migration: Rename ram_counters to mig_stats > >migration: Rename RAMStats to MigrationAtomicStats > >migration/rdma: Split the zero page case from acct_update_position > >migration/rdma: Unfold last user of acct_update_position() > >migration: Drop unused parameter for migration_tls_get_creds() > >migration: Drop unused parameter for migration_tls_client_create() > > > > Lukas Straub (13): > >qtest/migration-test.c: Add tests with compress enabled > >qtest/migration-test.c: Add postcopy tests with compress enabled > >ram.c: Let the compress threads return a CompressResult enum > >ram.c: Dont change param->block in the compress thread > >ram.c: Reset result after sending queued data > >ram.c: Do not call save_page_header() from compress threads > >ram.c: Call update_compress_thread_counts from > > compress_send_queued_data > >ram.c: Remove last ram.c dependency from the core compress code > >ram.c: Move core compression code into its own file > >ram.c: Move core decompression code into its own file > >ram compress: Assert that the file buffer matches the result > >ram-compress.c: Make target independent > >migration: Initialize and cleanup decompression in migration.c > > There are a bunch of migration failures in CI: > > https://gitlab.com/qemu-project/qemu/-/jobs/4201998343#L375 > https://gitlab.com/qemu-project/qemu/-/jobs/4201998342#L428 > https://gitlab.com/qemu-project/qemu/-/jobs/4201998340#L459 > https://gitlab.com/qemu-project/qemu/-/jobs/4201998336#L4883 > > > r~ Hmm, it doesn't always fail. Any way to get the testlog from the failed jobs? -- pgp4LdaxTgzzk.pgp Description: OpenPGP digital signature
RE: [PATCH v2 0/9] Hexagon (target/hexagon) New architecture support
> -Original Message- > From: Richard Henderson > Sent: Friday, April 28, 2023 4:45 PM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: phi...@linaro.org; a...@rev.ng; a...@rev.ng; Brian Cain > ; Matheus Bernardino (QUIC) > > Subject: Re: [PATCH v2 0/9] Hexagon (target/hexagon) New architecture > support > > WARNING: This email originated from outside of Qualcomm. Please be wary > of any links or attachments, and do not enable macros. > > On 4/27/23 23:40, Taylor Simpson wrote: > > Add support for new Hexagon architecture versions v68/v69/v71/v73 > > Where can one find docs for this? > The latest Hexagon SDK I can find is 3.5, which still ends at v67. I guess the folks at developer.qualcomm.com are behind in publishing specs. I'll work on getting these. Thanks, Taylor
RE: [PATCH v2] target/hexagon: fix = vs. == mishap
> -Original Message-
> From: Peter Maydell
> Sent: Saturday, April 29, 2023 4:27 AM
> To: Taylor Simpson
> Cc: qemu-devel@nongnu.org; pbonz...@redhat.com;
> richard.hender...@linaro.org; phi...@linaro.org; a...@rev.ng;
> a...@rev.ng; Brian Cain ; Matheus Bernardino (QUIC)
>
> Subject: Re: [PATCH v2] target/hexagon: fix = vs. == mishap
>
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
>
> On Fri, 28 Apr 2023 at 21:45, Taylor Simpson wrote:
> >
> > From: Paolo Bonzini
> >
> > Changes in v2
> > Fix yyassert's for sign and zero extends
> >
> > Coverity reports a parameter that is "set but never used". This is
> > caused by an assignment operator being used instead of equality.
>
> The commit message says it's fixing yyasserts, but the new changed code
> doesn't seem to be fixing yyasserts?
Hi Peter,
See below ...
Taylor
>
> > Co-authored-by: Taylor Simpson
> > Signed-off-by: Paolo Bonzini
> > Signed-off-by: Taylor Simpson
> > ---
> > target/hexagon/idef-parser/parser-helpers.c | 2 +-
> > target/hexagon/idef-parser/idef-parser.y| 4 ++--
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/hexagon/idef-parser/parser-helpers.c
> > b/target/hexagon/idef-parser/parser-helpers.c
> > index 86511efb62..0a01ec39b7 100644
> > --- a/target/hexagon/idef-parser/parser-helpers.c
> > +++ b/target/hexagon/idef-parser/parser-helpers.c
> > @@ -1123,7 +1123,7 @@ HexValue gen_extend_op(Context *c,
> > HexValue *value,
> > HexSignedness signedness) {
> > -unsigned bit_width = (dst_width = 64) ? 64 : 32;
> > +unsigned bit_width = (dst_width == 64) ? 64 : 32;
> > HexValue value_m = *value;
> > HexValue src_width_m = *src_width;
> >
Not that before Paolo's change, bit_width would always be set to 64. After the
change, it will be either 64 or 32.
The yyassert in question doesn't show up in the diff. It's just below the code
above
yyassert(c, locp, value_m.bit_width <= bit_width &&
src_width_m.bit_width <= bit_width,
"Extending to a size smaller than the current size"
" makes no sense");
There are some cases where the semantics code being parsed passes 33 as the
dst_width and a 64-bit value argument. So, the assert fails.
To fix the problem, we pass dst_width as 64 for the SXT (sign extend) and ZXT
(zero extend) productions below.
After this change, all call sites pass 64 as the value of that argument. So,
we could go even further and remove that parameter and always set bit_width to
64. Alessandro and Anton are more familiar with this code than I am, so I'll
wait for them to weigh in.
> > diff --git a/target/hexagon/idef-parser/idef-parser.y
> > b/target/hexagon/idef-parser/idef-parser.y
> > index 5444fd4749..2561f0ebb0 100644
> > --- a/target/hexagon/idef-parser/idef-parser.y
> > +++ b/target/hexagon/idef-parser/idef-parser.y
> > @@ -685,7 +685,7 @@ rvalue : FAIL
> > yyassert(c, &@1, $5.type == IMMEDIATE &&
> >$5.imm.type == VALUE,
> >"SXT expects immediate values\n");
> > - $$ = gen_extend_op(c, &@1, &$3, $5.imm.value, &$7, SIGNED);
> > + $$ = gen_extend_op(c, &@1, &$3, 64, &$7, SIGNED);
> > }
> > | ZXT '(' rvalue ',' IMM ',' rvalue ')'
> > {
> > @@ -693,7 +693,7 @@ rvalue : FAIL
> > yyassert(c, &@1, $5.type == IMMEDIATE &&
> >$5.imm.type == VALUE,
> >"ZXT expects immediate values\n");
> > - $$ = gen_extend_op(c, &@1, &$3, $5.imm.value, &$7, UNSIGNED);
> > + $$ = gen_extend_op(c, &@1, &$3, 64, &$7, UNSIGNED);
> > }
> > | '(' rvalue ')'
> > {
> > --
> > 2.25.1
>
> thanks
> -- PMM
RE: [PATCH v2] target/hexagon: fix = vs. == mishap
> -Original Message- > From: Paolo Bonzini > Sent: Saturday, April 29, 2023 7:24 AM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: richard.hender...@linaro.org; phi...@linaro.org; a...@rev.ng; > a...@rev.ng; Brian Cain ; Matheus Bernardino (QUIC) > > Subject: Re: [PATCH v2] target/hexagon: fix = vs. == mishap > > WARNING: This email originated from outside of Qualcomm. Please be wary > of any links or attachments, and do not enable macros. > > On 4/28/23 22:44, Taylor Simpson wrote: > > From: Paolo Bonzini > > > > Changes in v2 > > Fix yyassert's for sign and zero extends > > > > Coverity reports a parameter that is "set but never used". This is > > caused by an assignment operator being used instead of equality. > > > > Co-authored-by: Taylor Simpson > > Thanks for the fix Taylor, I'll let you submit this as part of your pull > requests. > > Paolo Will do. Thanks, Taylor
Re: [PULL 00/21] Migration 20230428 patches
On 4/28/23 20:11, Juan Quintela wrote: The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6: Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100) are available in the Git repository at: https://gitlab.com/juan.quintela/qemu.git tags/migration-20230428-pull-request for you to fetch changes up to 05ecac612ec6a4bdb358e68554b4406ac2c8e25a: migration: Initialize and cleanup decompression in migration.c (2023-04-28 20:54:53 +0200) Migration Pull request (20230429 vintage) Hi In this series: - compression code cleanup (lukas) nice, nice, nice. - drop useless parameters from migration_tls* (juan) - first part of remove QEMUFileHooks series (juan) Please apply. Juan Quintela (8): multifd: We already account for this packet on the multifd thread migration: Move ram_stats to its own file migration-stats.[ch] migration: Rename ram_counters to mig_stats migration: Rename RAMStats to MigrationAtomicStats migration/rdma: Split the zero page case from acct_update_position migration/rdma: Unfold last user of acct_update_position() migration: Drop unused parameter for migration_tls_get_creds() migration: Drop unused parameter for migration_tls_client_create() Lukas Straub (13): qtest/migration-test.c: Add tests with compress enabled qtest/migration-test.c: Add postcopy tests with compress enabled ram.c: Let the compress threads return a CompressResult enum ram.c: Dont change param->block in the compress thread ram.c: Reset result after sending queued data ram.c: Do not call save_page_header() from compress threads ram.c: Call update_compress_thread_counts from compress_send_queued_data ram.c: Remove last ram.c dependency from the core compress code ram.c: Move core compression code into its own file ram.c: Move core decompression code into its own file ram compress: Assert that the file buffer matches the result ram-compress.c: Make target independent migration: Initialize and cleanup decompression in migration.c There are a bunch of migration failures in CI: https://gitlab.com/qemu-project/qemu/-/jobs/4201998343#L375 https://gitlab.com/qemu-project/qemu/-/jobs/4201998342#L428 https://gitlab.com/qemu-project/qemu/-/jobs/4201998340#L459 https://gitlab.com/qemu-project/qemu/-/jobs/4201998336#L4883 r~
Re: [PATCH v4 3/3] Add a Hyper-V Dynamic Memory Protocol driver (hv-balloon)
On 29.04.2023 00:59, Philippe Mathieu-Daudé wrote: On 27/4/23 11:08, Maciej S. Szmigiero wrote: From: "Maciej S. Szmigiero" This driver is like virtio-balloon on steroids: it allows both changing the guest memory allocation via ballooning and inserting pieces of extra RAM into it on demand from a provided memory backend. One of advantages of these over ACPI-based PC DIMM hotplug is that such memory can be hotplugged in much smaller granularity because the ACPI DIMM slot limit does not apply. In order to enable hot-adding additional memory a new memory backend needs to be created and provided to the driver via the "memdev" parameter. This can be achieved by, for example, adding "-object memory-backend-ram,id=mem1,size=32G" to the QEMU command line and then instantiating the driver with "memdev=mem1" parameter. In contrast with ACPI DIMM hotplug where one can only request to unplug a whole DIMM stick this driver allows removing memory from guest in single page (4k) units via ballooning. The actual resizing is done via ballooning interface (for example, via the "balloon" HMP command) This includes resizing the guest past its boot size - that is, hot-adding additional memory in granularity limited only by the guest alignment requirements. After a VM reboot the guest is back to its original (boot) size. In the future, the guest boot memory size might be changed on reboot instead, taking into account the effective size that VM had before that reboot (much like Hyper-V does). For performance reasons, the guest-released memory is tracked in a few range trees, as a series of (start, count) ranges. Each time a new page range is inserted into such tree its neighbors are checked as candidates for possible merging with it. Besides performance reasons, the Dynamic Memory protocol itself uses page ranges as the data structure in its messages, so relevant pages need to be merged into such ranges anyway. One has to be careful when tracking the guest-released pages, since the guest can maliciously report returning pages outside its current address space, which later clash with the address range of newly added memory. Similarly, the guest can report freeing the same page twice. The above design results in much better ballooning performance than when using virtio-balloon with the same guest: 230 GB / minute with this driver versus 70 GB / minute with virtio-balloon. During a ballooning operation most of time is spent waiting for the guest to come up with newly freed page ranges, processing the received ranges on the host side (in QEMU and KVM) is nearly instantaneous. The unballoon operation is also pretty much instantaneous: thanks to the merging of the ballooned out page ranges 200 GB of memory can be returned to the guest in about 1 second. With virtio-balloon this operation takes about 2.5 minutes. These tests were done against a Windows Server 2019 guest running on a Xeon E5-2699, after dirtying the whole memory inside guest before each balloon operation. Using a range tree instead of a bitmap to track the removed memory also means that the solution scales well with the guest size: even a 1 TB range takes just a few bytes of such metadata. Since the required GTree operations aren't present in every Glib version a check for them was added to "configure" script, together with new "--enable-hv-balloon" and "--disable-hv-balloon" arguments. If these GTree operations are missing in the system's Glib version this driver will be skipped during QEMU build. An optional "status-report=on" device parameter requests memory status events from the guest (typically sent every second), which allow the host to learn both the guest memory available and the guest memory in use counts. They are emitted externally as "HV_BALLOON_STATUS_REPORT" QMP events. The driver is named hv-balloon since the Linux kernel client driver for the Dynamic Memory Protocol is named as such and to follow the naming pattern established by the virtio-balloon driver. The whole protocol runs over Hyper-V VMBus. The driver was tested against Windows Server 2012 R2, Windows Server 2016 and Windows Server 2019 guests and obeys the guest alignment requirements reported to the host via DM_CAPABILITIES_REPORT message. Signed-off-by: Maciej S. Szmigiero --- Kconfig.host | 3 + configure | 36 + hw/hyperv/Kconfig | 5 + hw/hyperv/hv-balloon.c | 2040 hw/hyperv/meson.build | 1 + hw/hyperv/trace-events | 16 + meson.build | 4 +- qapi/machine.json | 25 + 8 files changed, 2129 insertions(+), 1 deletion(-) create mode 100644 hw/hyperv/hv-balloon.c diff --git a/Kconfig.host b/Kconfig.host index d763d89269..2ee71578f3 100644 --- a/Kconfig.host +++ b/Kconfig.host @@ -46,3 +46,6 @@ config FUZZ config VFIO_USER_SERVER_ALLOWED bool imply VFIO_USER_SERVER + +config HV_BALLOON_POSSIBLE + bool Should this be restricted to litt
VNC clipboard support
Hello, I'm trying to use the copy/paste with VNC. I'm launching qemu with: $ qemu-system-x86_64 -hda debiandisk.img vnc :1 I'm using tightvncviewer which has support for copy/paste. I try to copy text between guest and host. It doesn't work. Neither from host to guest or guest to host. As far as I know, there is clipboard support in VNC (ui/vnc-clipboard.c and so on). With wireshark, I can see that tightvncviewer send clipboard requests. Am I missing some configuration? Note: I also tried to launch qemu with: $ qemu-system-x86_64 -hda debiandisk.img vnc :1 -device virtio-serial-pci -spice port=5930,disable-ticketing=on -device virtserialport,chardev=spicechannel0,name=com.redhat.spice.0 -chardev spicevmc,id=spicechannel0,name=vdagent but with no success. Questions: Does copy/paste works with VNC? (I'm beginning to think no?) Do I have to install something in the VM? (what?) Thank you
Re: [PATCH] tests/9p: fix potential leak in v9fs_rreaddir()
On Saturday, April 29, 2023 2:04:30 PM CEST Greg Kurz wrote:
> Hi Christian !
Hi there, it's been a while! :)
> On Sat, 29 Apr 2023 11:25:33 +0200
> Christian Schoenebeck wrote:
>
> > Free allocated directory entries in v9fs_rreaddir() if argument
> > `entries` was passed as NULL, to avoid a memory leak. It is
> > explicitly allowed by design for `entries` to be NULL. [1]
> >
> > [1] https://lore.kernel.org/all/1690923.g4PEXVpXuU@silver
> >
> > Reported-by: Coverity (CID 1487558)
> > Signed-off-by: Christian Schoenebeck
> > ---
>
> Good catch Coverity ! :-)
Yeah, this Coverity report is actually from March and I ignored it so far,
because the reported leak could never happen with current test code. But Paolo
brought it up this week, so ...
> Reviewed-by: Greg Kurz
>
> I still have a suggestion. See below.
>
> > tests/qtest/libqos/virtio-9p-client.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/tests/qtest/libqos/virtio-9p-client.c
> > b/tests/qtest/libqos/virtio-9p-client.c
> > index e4a368e036..b8adc8d4b9 100644
> > --- a/tests/qtest/libqos/virtio-9p-client.c
> > +++ b/tests/qtest/libqos/virtio-9p-client.c
> > @@ -594,6 +594,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count,
> > uint32_t *nentries,
> > {
> > uint32_t local_count;
> > struct V9fsDirent *e = NULL;
> > +/* only used to avoid a leak if entries was NULL */
> > +struct V9fsDirent *unused_entries = NULL;
> > uint16_t slen;
> > uint32_t n = 0;
> >
> > @@ -612,6 +614,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count,
> > uint32_t *nentries,
> > e = g_new(struct V9fsDirent, 1);
> > if (entries) {
> > *entries = e;
> > +} else {
> > +unused_entries = e;
> > }
> > } else {
> > e = e->next = g_new(struct V9fsDirent, 1);
>
> This is always allocating and chaining a new entry even
> though it isn't needed in the entries == NULL case.
>
> > @@ -628,6 +632,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count,
> > uint32_t *nentries,
> > *nentries = n;
> > }
> >
> > +v9fs_free_dirents(unused_entries);
>
> This is going to loop again on all entries to free them.
>
> > v9fs_req_free(req);
> > }
> >
>
> If this function is to be called one day with an enormous
> number of entries and entries == NULL case, this might
> not scale well.
>
> What about only allocating a single entry in this case ?
>
> E.g.
>
> @@ -593,7 +593,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
> *nentries,
> struct V9fsDirent **entries)
> {
> uint32_t local_count;
> -struct V9fsDirent *e = NULL;
> +g_autofree struct V9fsDirent *e = NULL;
> uint16_t slen;
> uint32_t n = 0;
>
> @@ -611,10 +611,12 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count,
> uint32_t *nentries,
> if (!e) {
> e = g_new(struct V9fsDirent, 1);
> if (entries) {
> -*entries = e;
> +*entries = g_steal_pointer(e);
g_steal_pointer(e) just sets `e` to NULL and returns its old value, so ...
> }
> } else {
> -e = e->next = g_new(struct V9fsDirent, 1);
> +if (entries) {
> +e = e->next = g_new(struct V9fsDirent, 1);
> +}
... this `else` block would never be reached and no list assembled.
> }
> e->next = NULL;
> /* qid[13] offset[8] type[1] name[s] */
And even if above's issue was fixed, then it would cause a use-after-free for
the last element in the list if entries != NULL and caller trying to access
the last element afterwards. So you would still need a separate g_autofree
pointer instead of tagging `e` directly, or something like this after loop
end:
if (entries)
g_steal_pointer(e);
Which would somehow defeat the purpose of using g_autofree though.
I mean, yes this could be addressed, but is it worth it? I don't know. Even
this reported leak is a purely theoretical one, but I understand if people
want to silence a warning.
Best regards,
Christian Schoenebeck
Re: [PATCH v3 06/57] tcg/i386: Generalize multi-part load overlap test
On 24/4/23 07:40, Richard Henderson wrote:
Test for both base and index; use datahi as a temporary, overwritten
by the final load. Always perform the loads in ascending order, so
that any (user-only) fault sees the correct address.
Signed-off-by: Richard Henderson
---
tcg/i386/tcg-target.c.inc | 31 +++
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
index b986109d77..794d440a9e 100644
--- a/tcg/i386/tcg-target.c.inc
+++ b/tcg/i386/tcg-target.c.inc
@@ -2223,23 +2223,22 @@ static void tcg_out_qemu_ld_direct(TCGContext *s,
TCGReg datalo, TCGReg datahi,
if (TCG_TARGET_REG_BITS == 64) {
tcg_out_modrm_sib_offset(s, movop + P_REXW + seg, datalo,
base, index, 0, ofs);
+break;
+}
+if (use_movbe) {
+TCGReg t = datalo;
+datalo = datahi;
+datahi = t;
+}
+if (base == datalo || index == datalo) {
+tcg_out_modrm_sib_offset(s, OPC_LEA, datahi, base, index, 0, ofs);
+tcg_out_modrm_offset(s, movop + seg, datalo, datahi, 0);
+tcg_out_modrm_offset(s, movop + seg, datahi, datahi, 4);
LGTM but I'd rather have someone fluent with x86 review this one...
} else {
-if (use_movbe) {
-TCGReg t = datalo;
-datalo = datahi;
-datahi = t;
-}
-if (base != datalo) {
-tcg_out_modrm_sib_offset(s, movop + seg, datalo,
- base, index, 0, ofs);
-tcg_out_modrm_sib_offset(s, movop + seg, datahi,
- base, index, 0, ofs + 4);
-} else {
-tcg_out_modrm_sib_offset(s, movop + seg, datahi,
- base, index, 0, ofs + 4);
-tcg_out_modrm_sib_offset(s, movop + seg, datalo,
- base, index, 0, ofs);
-}
+tcg_out_modrm_sib_offset(s, movop + seg, datalo,
+ base, index, 0, ofs);
+tcg_out_modrm_sib_offset(s, movop + seg, datahi,
+ base, index, 0, ofs + 4);
}
break;
default:
Re: [PATCH v3] meson: Pass -j option to sphinx
On 4/28/23 17:01, Fabiano Rosas wrote: Also make sure our plugins support parallelism and report it properly to sphinx. Particularly, implement the merge_domaindata method in DBusDomain that is used to merge in data from other subprocesses. before: $ time make man html ... [1/2] Generating docs/QEMU manual with a custom command [2/2] Generating docs/QEMU man pages with a custom command real0m43.157s user0m42.642s sys 0m0.576s after: $ time make man html ... [1/2] Generating docs/QEMU manual with a custom command [2/2] Generating docs/QEMU man pages with a custom command real0m25.014s user0m51.288s sys 0m2.085s The 'nproc' fallback will potentially cause twice #CPUs processes to be active, since sphinx will run in parallel with everything else. Is this result with "-j auto", and if so with which computer? If the speedup is only 2x as it seems to be from the "time" above, I'd rather have "-j 2" only so that sphinx doesn't risk killing the machine... Paolo
Re: [PATCH v3 54/57] tcg/ppc: Remove unused constraints A, B, C, D
On 24/4/23 07:41, Richard Henderson wrote:
These constraints have not been used for quite some time.
Fixes: 77b73de67632 ("Use rem/div[u]_i32 drop div[u]2_i32")
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target-con-str.h | 4
1 file changed, 4 deletions(-)
diff --git a/tcg/ppc/tcg-target-con-str.h b/tcg/ppc/tcg-target-con-str.h
index f3bf030bc3..9dcbc3df50 100644
--- a/tcg/ppc/tcg-target-con-str.h
+++ b/tcg/ppc/tcg-target-con-str.h
@@ -10,10 +10,6 @@
*/
REGS('r', ALL_GENERAL_REGS)
REGS('v', ALL_VECTOR_REGS)
-REGS('A', 1u << TCG_REG_R3)
-REGS('B', 1u << TCG_REG_R4)
-REGS('C', 1u << TCG_REG_R5)
-REGS('D', 1u << TCG_REG_R6)
Reviewed-by: Philippe Mathieu-Daudé
Is the J constraint introduced in commit 3d582c6179
("tcg-ppc64: Rearrange integer constant constraints")
ever used?
Re: [PATCH v3 0/2] accel/tcg/tcg-accel-ops-rr: ensure fairness with icount
On 29/4/23 11:28, Richard Henderson wrote: On 4/27/23 03:09, Jamie Iles wrote: From: Jamie Iles The round-robin scheduler will iterate over the CPU list with an assigned budget until the next timer expiry and may exit early because of a TB exit. This is fine under normal operation but with icount enabled and SMP it is possible for a CPU to be starved of run time and the system live-locks. For example, booting a riscv64 platform with '-icount shift=0,align=off,sleep=on -smp 2' we observe a livelock once the kernel has timers enabled and starts performing TLB shootdowns. In this case we have CPU 0 in M-mode with interrupts disabled sending an IPI to CPU 1. As we enter the TCG loop, we assign the icount budget to next timer interrupt to CPU 0 and begin executing where the guest is sat in a busy loop exhausting all of the budget before we try to execute CPU 1 which is the target of the IPI but CPU 1 is left with no budget with which to execute and the process repeats. We try here to add some fairness by splitting the budget across all of the CPUs on the thread fairly before entering each one. The CPU count is cached on CPU list generation ID to avoid iterating the list on each loop iteration. With this change it is possible to boot an SMP rv64 guest with icount enabled and no hangs. New in v3 (address feedback from Richard Henderson): - Additional patch to use QEMU_LOCK_GUARD with qemu_cpu_list_lock where appropriate - Move rr_cpu_count() call to be conditional on icount_enabled() - Initialize cpu_budget to 0 Jamie Iles (2): cpu: expose qemu_cpu_list_lock for lock-guard use accel/tcg/tcg-accel-ops-rr: ensure fairness with icount It appears as if one of these two patches causes a failure in replay, e.g. https://gitlab.com/rth7680/qemu/-/jobs/4200609234#L4162 Would you have a look, please? cpu count only going forward in rr_cpu_count()?
Re: [PATCH v2] target/hexagon: fix = vs. == mishap
On 4/28/23 22:44, Taylor Simpson wrote: From: Paolo Bonzini Changes in v2 Fix yyassert's for sign and zero extends Coverity reports a parameter that is "set but never used". This is caused by an assignment operator being used instead of equality. Co-authored-by: Taylor Simpson Thanks for the fix Taylor, I'll let you submit this as part of your pull requests. Paolo
[PULL v2 00/16] Misc patches for 2023-04-29
The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6: Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 42abcc584174166342297421209932a87bdb85f1: cpus-common: stop using mb_set/mb_read (2023-04-29 14:19:01 +0200) v1->v2: drop hexagon patch * Fix compilation issues under Debian 10 * Update kernel headers to 6.3rc5 * Suppress GCC13 false positive in aio_bh_poll() * Add new x86 feature bits * Coverity fixes * More steps towards removing qatomic_mb_set/read * Fix reduced-phys-bits value for AMD SEV Cédric Le Goater (1): async: Suppress GCC13 false positive in aio_bh_poll() David 'Digit' Turner (3): Fix libvhost-user.c compilation. update-linux-headers.sh: Add missing kernel headers. Update linux headers to v6.3rc5 Jiaxi Chen (6): target/i386: Add support for CMPCCXADD in CPUID enumeration target/i386: Add support for AMX-FP16 in CPUID enumeration target/i386: Add support for AVX-IFMA in CPUID enumeration target/i386: Add support for AVX-VNNI-INT8 in CPUID enumeration target/i386: Add support for AVX-NE-CONVERT in CPUID enumeration target/i386: Add support for PREFETCHIT0/1 in CPUID enumeration Paolo Bonzini (2): tests: vhost-user-test: release mutex on protocol violation cpus-common: stop using mb_set/mb_read Tom Lendacky (4): qapi, i386/sev: Change the reduced-phys-bits value from 5 to 1 qemu-options.hx: Update the reduced-phys-bits documentation i386/sev: Update checks and information related to reduced-phys-bits i386/cpu: Update how the EBX register of CPUID 0x801F is set cpus-common.c| 4 +- include/standard-headers/drm/drm_fourcc.h| 12 +++ include/standard-headers/linux/ethtool.h | 48 ++- include/standard-headers/linux/fuse.h| 45 ++- include/standard-headers/linux/pci_regs.h| 1 + include/standard-headers/linux/vhost_types.h | 2 + include/standard-headers/linux/virtio_blk.h | 105 linux-headers/asm-arm64/kvm.h| 1 + linux-headers/asm-x86/kvm.h | 34 +++- linux-headers/linux/const.h | 36 + linux-headers/linux/kvm.h| 9 +++ linux-headers/linux/memfd.h | 39 + linux-headers/linux/nvme_ioctl.h | 114 +++ linux-headers/linux/stddef.h | 47 +++ linux-headers/linux/vfio.h | 15 ++-- linux-headers/linux/vhost.h | 8 ++ qapi/misc-target.json| 2 +- qemu-options.hx | 4 +- scripts/update-linux-headers.sh | 4 +- subprojects/libvhost-user/libvhost-user.c| 6 ++ target/i386/cpu.c| 30 +-- target/i386/cpu.h| 14 target/i386/sev.c| 17 +++- tests/qtest/vhost-user-test.c| 3 +- util/async.c | 14 25 files changed, 588 insertions(+), 26 deletions(-) create mode 100644 linux-headers/linux/const.h create mode 100644 linux-headers/linux/memfd.h create mode 100644 linux-headers/linux/nvme_ioctl.h create mode 100644 linux-headers/linux/stddef.h -- 2.40.0
[PULL 10/17] target/i386: Add support for PREFETCHIT0/1 in CPUID enumeration
From: Jiaxi Chen
Latest Intel platform Granite Rapids has introduced a new instruction -
PREFETCHIT0/1, which moves code to memory (cache) closer to the
processor depending on specific hints.
The bit definition:
CPUID.(EAX=7,ECX=1):EDX[bit 14]
Add CPUID definition for PREFETCHIT0/1.
Signed-off-by: Jiaxi Chen
Signed-off-by: Tao Su
Reviewed-by: Xiaoyao Li
Message-Id: <20230303065913.1246327-7-tao1...@linux.intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0204a3ac801a..823320fe420c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -897,7 +897,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
"avx-vnni-int8", "avx-ne-convert", NULL, NULL,
NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
+NULL, NULL, "prefetchiti", NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b46d52f3fa44..8504aaac6807 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -925,6 +925,8 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
#define CPUID_7_1_EDX_AVX_VNNI_INT8 (1U << 4)
/* AVX NE CONVERT Instructions */
#define CPUID_7_1_EDX_AVX_NE_CONVERT(1U << 5)
+/* PREFETCHIT0/1 Instructions */
+#define CPUID_7_1_EDX_PREFETCHITI (1U << 14)
/* XFD Extend Feature Disabled */
#define CPUID_D_1_EAX_XFD (1U << 4)
--
2.40.0
[PULL 11/17] Fix libvhost-user.c compilation.
From: David 'Digit' Turner The source file uses VIRTIO_F_VERSION_1 which is not defined by on Debian 10. The system-provided which does not include the macro definition is included through , so fix the issue by including the standard-headers version before that. Signed-off-by: David 'Digit' Turner Message-Id: <20230405125920.2951721-2-di...@google.com> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index 0abd898a52c4..8fb61e2df2fe 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -32,6 +32,12 @@ #include #include +/* Necessary to provide VIRTIO_F_VERSION_1 on system + * with older linux headers. Must appear before + * below. + */ +#include "standard-headers/linux/virtio_config.h" + #if defined(__linux__) #include #include -- 2.40.0
[PULL 06/17] target/i386: Add support for AMX-FP16 in CPUID enumeration
From: Jiaxi Chen
Latest Intel platform Granite Rapids has introduced a new instruction -
AMX-FP16, which performs dot-products of two FP16 tiles and accumulates
the results into a packed single precision tile. AMX-FP16 adds FP16
capability and allows a FP16 GPU trained model to run faster without
loss of accuracy or added SW overhead.
The bit definition:
CPUID.(EAX=7,ECX=1):EAX[bit 21]
Add CPUID definition for AMX-FP16.
Signed-off-by: Jiaxi Chen
Signed-off-by: Tao Su
Reviewed-by: Xiaoyao Li
Message-Id: <20230303065913.1246327-3-tao1...@linux.intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 67210ffd79b9..841c407d6d76 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -879,7 +879,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, "fzrm", "fsrs",
"fsrc", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
+NULL, "amx-fp16", NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d5843c15558f..7deb37eca5a8 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -915,6 +915,8 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
#define CPUID_7_1_EAX_FSRS (1U << 11)
/* Fast Short REP CMPS/SCAS */
#define CPUID_7_1_EAX_FSRC (1U << 12)
+/* Support Tile Computational Operations on FP16 Numbers */
+#define CPUID_7_1_EAX_AMX_FP16 (1U << 21)
/* XFD Extend Feature Disabled */
#define CPUID_D_1_EAX_XFD (1U << 4)
--
2.40.0
[PULL 05/17] target/i386: Add support for CMPCCXADD in CPUID enumeration
From: Jiaxi Chen
CMPccXADD is a new set of instructions in the latest Intel platform
Sierra Forest. This new instruction set includes a semaphore operation
that can compare and add the operands if condition is met, which can
improve database performance.
The bit definition:
CPUID.(EAX=7,ECX=1):EAX[bit 7]
Add CPUID definition for CMPCCXADD.
Signed-off-by: Jiaxi Chen
Signed-off-by: Tao Su
Reviewed-by: Xiaoyao Li
Message-Id: <20230303065913.1246327-2-tao1...@linux.intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 73dd99374abe..67210ffd79b9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -875,7 +875,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, NULL, NULL,
-"avx-vnni", "avx512-bf16", NULL, NULL,
+"avx-vnni", "avx512-bf16", NULL, "cmpccxadd",
NULL, NULL, "fzrm", "fsrs",
"fsrc", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d243e290d385..d5843c15558f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -907,6 +907,8 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
#define CPUID_7_1_EAX_AVX_VNNI (1U << 4)
/* AVX512 BFloat16 Instruction */
#define CPUID_7_1_EAX_AVX512_BF16 (1U << 5)
+/* CMPCCXADD Instructions */
+#define CPUID_7_1_EAX_CMPCCXADD (1U << 7)
/* Fast Zero REP MOVS */
#define CPUID_7_1_EAX_FZRM (1U << 10)
/* Fast Short REP STOS */
--
2.40.0
[PULL 09/17] target/i386: Add support for AVX-NE-CONVERT in CPUID enumeration
From: Jiaxi Chen
AVX-NE-CONVERT is a new set of instructions which can convert low
precision floating point like BF16/FP16 to high precision floating point
FP32, as well as convert FP32 elements to BF16. This instruction allows
the platform to have improved AI capabilities and better compatibility.
The bit definition:
CPUID.(EAX=7,ECX=1):EDX[bit 5]
Add CPUID definition for AVX-NE-CONVERT.
Signed-off-by: Jiaxi Chen
Signed-off-by: Tao Su
Reviewed-by: Xiaoyao Li
Message-Id: <20230303065913.1246327-6-tao1...@linux.intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index abceab2b6992..0204a3ac801a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -895,7 +895,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, NULL, NULL,
-"avx-vnni-int8", NULL, NULL, NULL,
+"avx-vnni-int8", "avx-ne-convert", NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0b25d180753b..b46d52f3fa44 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -923,6 +923,8 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
/* Support for VPDPB[SU,UU,SS]D[,S] */
#define CPUID_7_1_EDX_AVX_VNNI_INT8 (1U << 4)
+/* AVX NE CONVERT Instructions */
+#define CPUID_7_1_EDX_AVX_NE_CONVERT(1U << 5)
/* XFD Extend Feature Disabled */
#define CPUID_D_1_EAX_XFD (1U << 4)
--
2.40.0
[PULL 17/17] cpus-common: stop using mb_set/mb_read
Use a store-release at the end of the work item, and a load-acquire when
waiting for the item to be completed. This is the standard message passing
pattern and is both enough and clearer than mb_read/mb_set.
Signed-off-by: Paolo Bonzini
---
cpus-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index b0047e456f93..a53716deb437 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -157,7 +157,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func,
run_on_cpu_data data,
wi.exclusive = false;
queue_work_on_cpu(cpu, &wi);
-while (!qatomic_mb_read(&wi.done)) {
+while (!qatomic_load_acquire(&wi.done)) {
CPUState *self_cpu = current_cpu;
qemu_cond_wait(&qemu_work_cond, mutex);
@@ -363,7 +363,7 @@ void process_queued_cpu_work(CPUState *cpu)
if (wi->free) {
g_free(wi);
} else {
-qatomic_mb_set(&wi->done, true);
+qatomic_store_release(&wi->done, true);
}
}
qemu_mutex_unlock(&cpu->work_mutex);
--
2.40.0
[PULL 13/17] Update linux headers to v6.3rc5
From: David 'Digit' Turner
commit 7e364e56293bb98cae1b55fd835f5991c4e96e7d
Signed-off-by: David 'Digit' Turner
Message-Id: <20230405172109.3081788-4-di...@google.com>
Signed-off-by: Paolo Bonzini
---
include/standard-headers/drm/drm_fourcc.h| 12 ++
include/standard-headers/linux/ethtool.h | 48 +++-
include/standard-headers/linux/fuse.h| 45 +++-
include/standard-headers/linux/pci_regs.h| 1 +
include/standard-headers/linux/vhost_types.h | 2 +
include/standard-headers/linux/virtio_blk.h | 105 +
linux-headers/asm-arm64/kvm.h| 1 +
linux-headers/asm-x86/kvm.h | 34 +-
linux-headers/linux/const.h | 36 ++
linux-headers/linux/kvm.h| 9 ++
linux-headers/linux/memfd.h | 39 +++
linux-headers/linux/nvme_ioctl.h | 114 +++
linux-headers/linux/stddef.h | 47
linux-headers/linux/vfio.h | 15 ++-
linux-headers/linux/vhost.h | 8 ++
15 files changed, 506 insertions(+), 10 deletions(-)
create mode 100644 linux-headers/linux/const.h
create mode 100644 linux-headers/linux/memfd.h
create mode 100644 linux-headers/linux/nvme_ioctl.h
create mode 100644 linux-headers/linux/stddef.h
diff --git a/include/standard-headers/drm/drm_fourcc.h
b/include/standard-headers/drm/drm_fourcc.h
index 69cab17b383f..dc3e6112c11c 100644
--- a/include/standard-headers/drm/drm_fourcc.h
+++ b/include/standard-headers/drm/drm_fourcc.h
@@ -87,6 +87,18 @@ extern "C" {
*
* The authoritative list of format modifier codes is found in
* `include/uapi/drm/drm_fourcc.h`
+ *
+ * Open Source User Waiver
+ * ---
+ *
+ * Because this is the authoritative source for pixel formats and modifiers
+ * referenced by GL, Vulkan extensions and other standards and hence used both
+ * by open source and closed source driver stacks, the usual requirement for an
+ * upstream in-kernel or open source userspace user does not apply.
+ *
+ * To ensure, as much as feasible, compatibility across stacks and avoid
+ * confusion with incompatible enumerations stakeholders for all relevant
driver
+ * stacks should approve additions.
*/
#define fourcc_code(a, b, c, d) ((uint32_t)(a) | ((uint32_t)(b) << 8) | \
diff --git a/include/standard-headers/linux/ethtool.h
b/include/standard-headers/linux/ethtool.h
index 87176ab075d2..99fcddf04f88 100644
--- a/include/standard-headers/linux/ethtool.h
+++ b/include/standard-headers/linux/ethtool.h
@@ -711,6 +711,24 @@ enum ethtool_stringset {
ETH_SS_COUNT
};
+/**
+ * enum ethtool_mac_stats_src - source of ethtool MAC statistics
+ * @ETHTOOL_MAC_STATS_SRC_AGGREGATE:
+ * if device supports a MAC merge layer, this retrieves the aggregate
+ * statistics of the eMAC and pMAC. Otherwise, it retrieves just the
+ * statistics of the single (express) MAC.
+ * @ETHTOOL_MAC_STATS_SRC_EMAC:
+ * if device supports a MM layer, this retrieves the eMAC statistics.
+ * Otherwise, it retrieves the statistics of the single (express) MAC.
+ * @ETHTOOL_MAC_STATS_SRC_PMAC:
+ * if device supports a MM layer, this retrieves the pMAC statistics.
+ */
+enum ethtool_mac_stats_src {
+ ETHTOOL_MAC_STATS_SRC_AGGREGATE,
+ ETHTOOL_MAC_STATS_SRC_EMAC,
+ ETHTOOL_MAC_STATS_SRC_PMAC,
+};
+
/**
* enum ethtool_module_power_mode_policy - plug-in module power mode policy
* @ETHTOOL_MODULE_POWER_MODE_POLICY_HIGH: Module is always in high power mode.
@@ -779,6 +797,31 @@ enum ethtool_podl_pse_pw_d_status {
ETHTOOL_PODL_PSE_PW_D_STATUS_ERROR,
};
+/**
+ * enum ethtool_mm_verify_status - status of MAC Merge Verify function
+ * @ETHTOOL_MM_VERIFY_STATUS_UNKNOWN:
+ * verification status is unknown
+ * @ETHTOOL_MM_VERIFY_STATUS_INITIAL:
+ * the 802.3 Verify State diagram is in the state INIT_VERIFICATION
+ * @ETHTOOL_MM_VERIFY_STATUS_VERIFYING:
+ * the Verify State diagram is in the state VERIFICATION_IDLE,
+ * SEND_VERIFY or WAIT_FOR_RESPONSE
+ * @ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED:
+ * indicates that the Verify State diagram is in the state VERIFIED
+ * @ETHTOOL_MM_VERIFY_STATUS_FAILED:
+ * the Verify State diagram is in the state VERIFY_FAIL
+ * @ETHTOOL_MM_VERIFY_STATUS_DISABLED:
+ * verification of preemption operation is disabled
+ */
+enum ethtool_mm_verify_status {
+ ETHTOOL_MM_VERIFY_STATUS_UNKNOWN,
+ ETHTOOL_MM_VERIFY_STATUS_INITIAL,
+ ETHTOOL_MM_VERIFY_STATUS_VERIFYING,
+ ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED,
+ ETHTOOL_MM_VERIFY_STATUS_FAILED,
+ ETHTOOL_MM_VERIFY_STATUS_DISABLED,
+};
+
/**
* struct ethtool_gstrings - string set for data tagging
* @cmd: Command number = %ETHTOOL_GSTRINGS
@@ -1183,7 +1226,7 @@ struct ethtool_rxnfc {
uint32_trule_cnt;
uint32_trss_context;
[PULL 15/17] target/hexagon: fix = vs. == mishap
Coverity reports a parameter that is "set but never used". This is caused
by an assignment operator being used instead of equality.
Cc: Taylor Simpson
Signed-off-by: Paolo Bonzini
---
target/hexagon/idef-parser/parser-helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/hexagon/idef-parser/parser-helpers.c
b/target/hexagon/idef-parser/parser-helpers.c
index 86511efb62b9..0a01ec39b75e 100644
--- a/target/hexagon/idef-parser/parser-helpers.c
+++ b/target/hexagon/idef-parser/parser-helpers.c
@@ -1123,7 +1123,7 @@ HexValue gen_extend_op(Context *c,
HexValue *value,
HexSignedness signedness)
{
-unsigned bit_width = (dst_width = 64) ? 64 : 32;
+unsigned bit_width = (dst_width == 64) ? 64 : 32;
HexValue value_m = *value;
HexValue src_width_m = *src_width;
--
2.40.0
[PULL 14/17] tests: vhost-user-test: release mutex on protocol violation
chr_read() is printing an error message and returning with s->data_mutex taken.
This can potentially cause a hang. Reported by Coverity.
Signed-off-by: Paolo Bonzini
---
tests/qtest/vhost-user-test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index bf9f7c4248ca..e4f95b2858f0 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -351,7 +351,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int
size)
if (size != msg.size) {
qos_printf("%s: Wrong message size received %d != %d\n",
__func__, size, msg.size);
-return;
+goto out;
}
}
@@ -509,6 +509,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int
size)
break;
}
+out:
g_mutex_unlock(&s->data_mutex);
}
--
2.40.0
[PULL 12/17] update-linux-headers.sh: Add missing kernel headers.
From: David 'Digit' Turner Add , used by hw/display/virtio-gpu-udmabuf.c Add , used by qga/commands-posix.c Add used by kvm-all.c, which requires the _BITUL() macro definition to be available. Without these, QEMU will not compile on Debian 10 systems. Signed-off-by: David 'Digit' Turner Message-Id: <20230405172109.3081788-3-di...@google.com> [Add for __DECLARE_FLEX_ARRAY. - Paolo] Signed-off-by: Paolo Bonzini --- scripts/update-linux-headers.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index b1ad99cba824..35a64bb50193 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -160,8 +160,8 @@ done rm -rf "$output/linux-headers/linux" mkdir -p "$output/linux-headers/linux" -for header in kvm.h vfio.h vfio_ccw.h vfio_zdev.h vhost.h \ - psci.h psp-sev.h userfaultfd.h mman.h vduse.h; do +for header in const.h stddef.h kvm.h vfio.h vfio_ccw.h vfio_zdev.h vhost.h \ + psci.h psp-sev.h userfaultfd.h memfd.h mman.h nvme_ioctl.h vduse.h; do cp "$tmpdir/include/linux/$header" "$output/linux-headers/linux" done -- 2.40.0
[PULL 16/17] async: Suppress GCC13 false positive in aio_bh_poll()
From: Cédric Le Goater
GCC13 reports an error :
../util/async.c: In function ‘aio_bh_poll’:
include/qemu/queue.h:303:22: error: storing the address of local variable
‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ [-Werror=dangling-pointer=]
303 | (head)->sqh_last = &(elm)->field.sqe_next;
\
| ~^~~~
../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’
169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
| ^~~~
../util/async.c:161:17: note: ‘slice’ declared here
161 | BHListSlice slice;
| ^
../util/async.c:161:17: note: ‘ctx’ declared here
But the local variable 'slice' is removed from the global context list
in following loop of the same routine. Add a pragma to silent GCC.
Cc: Stefan Hajnoczi
Cc: Paolo Bonzini
Cc: Daniel P. Berrangé
Signed-off-by: Cédric Le Goater
Reviewed-by: Daniel Henrique Barboza
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Thomas Huth
Tested-by: Daniel Henrique Barboza
Message-Id: <20230420202939.1982044-1-...@kaod.org>
Signed-off-by: Paolo Bonzini
---
util/async.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/util/async.c b/util/async.c
index 21016a1ac7c1..856e1a8a337a 100644
--- a/util/async.c
+++ b/util/async.c
@@ -164,7 +164,21 @@ int aio_bh_poll(AioContext *ctx)
/* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
+
+/*
+ * GCC13 [-Werror=dangling-pointer=] complains that the local variable
+ * 'slice' is being stored in the global 'ctx->bh_slice_list' but the
+ * list is emptied before this function returns.
+ */
+#if !defined(__clang__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wpragmas"
+#pragma GCC diagnostic ignored "-Wdangling-pointer="
+#endif
QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
+#if !defined(__clang__)
+#pragma GCC diagnostic pop
+#endif
while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
QEMUBH *bh;
--
2.40.0
[PULL 04/17] i386/cpu: Update how the EBX register of CPUID 0x8000001F is set
From: Tom Lendacky
Update the setting of CPUID 0x801F EBX to clearly document the ranges
associated with fields being set.
Fixes: 6cb8f2a663 ("cpu/i386: populate CPUID 0x8000_001F when SEV is active")
Signed-off-by: Tom Lendacky
Reviewed-by: Dr. David Alan Gilbert
Message-Id:
<5822fd7d02b575121380e1f493a8f6d9eba2b11a.1664550870.git.thomas.lenda...@amd.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2e30e348a176..73dd99374abe 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6000,8 +6000,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
if (sev_enabled()) {
*eax = 0x2;
*eax |= sev_es_enabled() ? 0x8 : 0;
-*ebx = sev_get_cbit_position();
-*ebx |= sev_get_reduced_phys_bits() << 6;
+*ebx = sev_get_cbit_position() & 0x3f; /* EBX[5:0] */
+*ebx |= (sev_get_reduced_phys_bits() & 0x3f) << 6; /* EBX[11:6] */
}
break;
default:
--
2.40.0
[PULL 03/17] i386/sev: Update checks and information related to reduced-phys-bits
From: Tom Lendacky
The value of the reduced-phys-bits parameter is propogated to the CPUID
information exposed to the guest. Update the current validation check to
account for the size of the CPUID field (6-bits), ensuring the value is
in the range of 1 to 63.
Maintain backward compatibility, to an extent, by allowing a value greater
than 1 (so that the previously documented value of 5 still works), but not
allowing anything over 63.
Fixes: d8575c6c02 ("sev/i386: add command to initialize the memory encryption
context")
Signed-off-by: Tom Lendacky
Reviewed-by: Dr. David Alan Gilbert
Message-Id:
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 859e06f6ad77..fe2144c0388b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -932,15 +932,26 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error
**errp)
host_cpuid(0x801F, 0, NULL, &ebx, NULL, NULL);
host_cbitpos = ebx & 0x3f;
+/*
+ * The cbitpos value will be placed in bit positions 5:0 of the EBX
+ * register of CPUID 0x801F. No need to verify the range as the
+ * comparison against the host value accomplishes that.
+ */
if (host_cbitpos != sev->cbitpos) {
error_setg(errp, "%s: cbitpos check failed, host '%d' requested '%d'",
__func__, host_cbitpos, sev->cbitpos);
goto err;
}
-if (sev->reduced_phys_bits < 1) {
-error_setg(errp, "%s: reduced_phys_bits check failed, it should be
>=1,"
- " requested '%d'", __func__, sev->reduced_phys_bits);
+/*
+ * The reduced-phys-bits value will be placed in bit positions 11:6 of
+ * the EBX register of CPUID 0x801F, so verify the supplied value
+ * is in the range of 1 to 63.
+ */
+if (sev->reduced_phys_bits < 1 || sev->reduced_phys_bits > 63) {
+error_setg(errp, "%s: reduced_phys_bits check failed,"
+ " it should be in the range of 1 to 63, requested '%d'",
+ __func__, sev->reduced_phys_bits);
goto err;
}
--
2.40.0
[PULL 07/17] target/i386: Add support for AVX-IFMA in CPUID enumeration
From: Jiaxi Chen
AVX-IFMA is a new instruction in the latest Intel platform Sierra
Forest. This instruction packed multiplies unsigned 52-bit integers and
adds the low/high 52-bit products to Qword Accumulators.
The bit definition:
CPUID.(EAX=7,ECX=1):EAX[bit 23]
Add CPUID definition for AVX-IFMA.
Signed-off-by: Jiaxi Chen
Signed-off-by: Tao Su
Reviewed-by: Xiaoyao Li
Message-Id: <20230303065913.1246327-4-tao1...@linux.intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 841c407d6d76..8eb2ee5045d7 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -879,7 +879,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, "fzrm", "fsrs",
"fsrc", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
-NULL, "amx-fp16", NULL, NULL,
+NULL, "amx-fp16", NULL, "avx-ifma",
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 7deb37eca5a8..1f72d11e0ccc 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -917,6 +917,8 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
#define CPUID_7_1_EAX_FSRC (1U << 12)
/* Support Tile Computational Operations on FP16 Numbers */
#define CPUID_7_1_EAX_AMX_FP16 (1U << 21)
+/* Support for VPMADD52[H,L]UQ */
+#define CPUID_7_1_EAX_AVX_IFMA (1U << 23)
/* XFD Extend Feature Disabled */
#define CPUID_D_1_EAX_XFD (1U << 4)
--
2.40.0
[PULL 00/17] Misc patches for 2023-04-29
The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6: Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to f40288a1f22acbf0ff6b08c192fa993e4af935ef: cpus-common: stop using mb_set/mb_read (2023-04-28 15:56:42 +0200) * Fix compilation issues under Debian 10 * Update kernel headers to 6.3rc5 * Suppress GCC13 false positive in aio_bh_poll() * Add new x86 feature bits * Coverity fixes * More steps towards removing qatomic_mb_set/read * Fix reduced-phys-bits value for AMD SEV Cédric Le Goater (1): async: Suppress GCC13 false positive in aio_bh_poll() David 'Digit' Turner (3): Fix libvhost-user.c compilation. update-linux-headers.sh: Add missing kernel headers. Update linux headers to v6.3rc5 Jiaxi Chen (6): target/i386: Add support for CMPCCXADD in CPUID enumeration target/i386: Add support for AMX-FP16 in CPUID enumeration target/i386: Add support for AVX-IFMA in CPUID enumeration target/i386: Add support for AVX-VNNI-INT8 in CPUID enumeration target/i386: Add support for AVX-NE-CONVERT in CPUID enumeration target/i386: Add support for PREFETCHIT0/1 in CPUID enumeration Paolo Bonzini (3): tests: vhost-user-test: release mutex on protocol violation target/hexagon: fix = vs. == mishap cpus-common: stop using mb_set/mb_read Tom Lendacky (4): qapi, i386/sev: Change the reduced-phys-bits value from 5 to 1 qemu-options.hx: Update the reduced-phys-bits documentation i386/sev: Update checks and information related to reduced-phys-bits i386/cpu: Update how the EBX register of CPUID 0x801F is set cpus-common.c| 4 +- include/standard-headers/drm/drm_fourcc.h| 12 +++ include/standard-headers/linux/ethtool.h | 48 ++- include/standard-headers/linux/fuse.h| 45 ++- include/standard-headers/linux/pci_regs.h| 1 + include/standard-headers/linux/vhost_types.h | 2 + include/standard-headers/linux/virtio_blk.h | 105 linux-headers/asm-arm64/kvm.h| 1 + linux-headers/asm-x86/kvm.h | 34 +++- linux-headers/linux/const.h | 36 + linux-headers/linux/kvm.h| 9 +++ linux-headers/linux/memfd.h | 39 + linux-headers/linux/nvme_ioctl.h | 114 +++ linux-headers/linux/stddef.h | 47 +++ linux-headers/linux/vfio.h | 15 ++-- linux-headers/linux/vhost.h | 8 ++ qapi/misc-target.json| 2 +- qemu-options.hx | 4 +- scripts/update-linux-headers.sh | 4 +- subprojects/libvhost-user/libvhost-user.c| 6 ++ target/hexagon/idef-parser/parser-helpers.c | 2 +- target/i386/cpu.c| 30 +-- target/i386/cpu.h| 14 target/i386/sev.c| 17 +++- tests/qtest/vhost-user-test.c| 3 +- util/async.c | 14 26 files changed, 589 insertions(+), 27 deletions(-) create mode 100644 linux-headers/linux/const.h create mode 100644 linux-headers/linux/memfd.h create mode 100644 linux-headers/linux/nvme_ioctl.h create mode 100644 linux-headers/linux/stddef.h -- 2.40.0
[PULL 02/17] qemu-options.hx: Update the reduced-phys-bits documentation
From: Tom Lendacky
A guest only ever experiences, at most, 1 bit of reduced physical
addressing. Update the documentation to reflect this as well as change
the example value on the reduced-phys-bits option.
Fixes: a9b4942f48 ("target/i386: add Secure Encrypted Virtualization (SEV)
object")
Signed-off-by: Tom Lendacky
Reviewed-by: Dr. David Alan Gilbert
Message-Id:
<13a62ced1808546c1d398e2025cf85f4c94ae123.1664550870.git.thomas.lenda...@amd.com>
Signed-off-by: Paolo Bonzini
---
qemu-options.hx | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index b5efa648bad1..42fc90aae473 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -5438,7 +5438,7 @@ SRST
physical address space. The ``reduced-phys-bits`` is used to
provide the number of bits we loose in physical address space.
Similar to C-bit, the value is Host family dependent. On EPYC,
-the value should be 5.
+a guest will lose a maximum of 1 bit, so the value should be 1.
The ``sev-device`` provides the device file to use for
communicating with the SEV firmware running inside AMD Secure
@@ -5473,7 +5473,7 @@ SRST
# |qemu_system_x86| \\
.. \\
- -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 \\
+ -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1 \\
-machine ...,memory-encryption=sev0 \\
.
--
2.40.0
[PULL 08/17] target/i386: Add support for AVX-VNNI-INT8 in CPUID enumeration
From: Jiaxi Chen
AVX-VNNI-INT8 is a new set of instructions in the latest Intel platform
Sierra Forest, aims for the platform to have superior AI capabilities.
This instruction multiplies the individual bytes of two unsigned or
unsigned source operands, then adds and accumulates the results into the
destination dword element size operand.
The bit definition:
CPUID.(EAX=7,ECX=1):EDX[bit 4]
AVX-VNNI-INT8 is on a new feature bits leaf. Add a CPUID feature word
FEAT_7_1_EDX for this leaf.
Add CPUID definition for AVX-VNNI-INT8.
Signed-off-by: Jiaxi Chen
Signed-off-by: Tao Su
Reviewed-by: Xiaoyao Li
Message-Id: <20230303065913.1246327-5-tao1...@linux.intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 22 +-
target/i386/cpu.h | 4
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8eb2ee5045d7..abceab2b6992 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -667,6 +667,7 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
#define TCG_7_0_EDX_FEATURES CPUID_7_0_EDX_FSRM
#define TCG_7_1_EAX_FEATURES (CPUID_7_1_EAX_FZRM | CPUID_7_1_EAX_FSRS | \
CPUID_7_1_EAX_FSRC)
+#define TCG_7_1_EDX_FEATURES 0
#define TCG_APM_FEATURES 0
#define TCG_6_EAX_FEATURES CPUID_6_EAX_ARAT
#define TCG_XSAVE_FEATURES (CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1)
@@ -890,6 +891,25 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.tcg_features = TCG_7_1_EAX_FEATURES,
},
+[FEAT_7_1_EDX] = {
+.type = CPUID_FEATURE_WORD,
+.feat_names = {
+NULL, NULL, NULL, NULL,
+"avx-vnni-int8", NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+},
+.cpuid = {
+.eax = 7,
+.needs_ecx = true, .ecx = 1,
+.reg = R_EDX,
+},
+.tcg_features = TCG_7_1_EDX_FEATURES,
+},
[FEAT_8000_0007_EDX] = {
.type = CPUID_FEATURE_WORD,
.feat_names = {
@@ -5534,9 +5554,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
}
} else if (count == 1) {
*eax = env->features[FEAT_7_1_EAX];
+*edx = env->features[FEAT_7_1_EDX];
*ebx = 0;
*ecx = 0;
-*edx = 0;
} else {
*eax = 0;
*ebx = 0;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1f72d11e0ccc..0b25d180753b 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -626,6 +626,7 @@ typedef enum FeatureWord {
FEAT_SGX_12_1_EAX, /* CPUID[EAX=0x12,ECX=1].EAX (SGX ATTRIBUTES[31:0]) */
FEAT_XSAVE_XSS_LO, /* CPUID[EAX=0xd,ECX=1].ECX */
FEAT_XSAVE_XSS_HI, /* CPUID[EAX=0xd,ECX=1].EDX */
+FEAT_7_1_EDX, /* CPUID[EAX=7,ECX=1].EDX */
FEATURE_WORDS,
} FeatureWord;
@@ -920,6 +921,9 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
/* Support for VPMADD52[H,L]UQ */
#define CPUID_7_1_EAX_AVX_IFMA (1U << 23)
+/* Support for VPDPB[SU,UU,SS]D[,S] */
+#define CPUID_7_1_EDX_AVX_VNNI_INT8 (1U << 4)
+
/* XFD Extend Feature Disabled */
#define CPUID_D_1_EAX_XFD (1U << 4)
--
2.40.0
[PULL 01/17] qapi, i386/sev: Change the reduced-phys-bits value from 5 to 1
From: Tom Lendacky
A guest only ever experiences, at most, 1 bit of reduced physical
addressing. Change the query-sev-capabilities json comment to use 1.
Fixes: 31dd67f684 ("sev/i386: qmp: add query-sev-capabilities command")
Signed-off-by: Tom Lendacky
Reviewed-by: Dr. David Alan Gilbert
Message-Id:
Signed-off-by: Paolo Bonzini
---
qapi/misc-target.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index de9105452377..bf04042f45d9 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -172,7 +172,7 @@
# -> { "execute": "query-sev-capabilities" }
# <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
# "cpu0-id": "2lvmGwo+...61iEinw==",
-# "cbitpos": 47, "reduced-phys-bits": 5}}
+# "cbitpos": 47, "reduced-phys-bits": 1}}
#
##
{ 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
--
2.40.0
Re: [PATCH] hw/remote: Fix vfu_cfg trace offset format
On 26/4/23 11:35, Mattias Nissler wrote: The printed offset value is prefixed with 0x, but was actually printed in decimal. To spare others the confusion, adjust the format specifier to hexadecimal. Signed-off-by: Mattias Nissler --- hw/remote/trace-events | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 1/9] hw/ide/ahci: remove stray backslash
On 28/4/23 15:21, Niklas Cassel wrote: From: Niklas Cassel This backslash obviously does not belong here, so remove it. Signed-off-by: Niklas Cassel --- hw/ide/ahci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] hw/nvram: Avoid unnecessary Xilinx eFuse backstore write
On 26/4/23 23:16, Tong Ho wrote: Add a check in the bit-set operation to write the backstore only if the affected bit is 0 before. With this in place, there will be no need for callers to do the checking in order to avoid unnecessary writes. Signed-off-by: Tong Ho --- hw/nvram/xlnx-efuse.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 06/21] Hexagon (target/hexagon) Remove log_reg_write from op_helper.[ch]
On 26/4/23 02:41, Taylor Simpson wrote: With the overrides added in prior commits, this function is not used Remove references in macros.h Signed-off-by: Taylor Simpson --- target/hexagon/macros.h| 14 -- target/hexagon/op_helper.h | 4 target/hexagon/op_helper.c | 17 - 3 files changed, 35 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 1/2] cpu: expose qemu_cpu_list_lock for lock-guard use
On 27/4/23 04:09, Jamie Iles wrote: Expose qemu_cpu_list_lock globally so that we can use WITH_QEMU_LOCK_GUARD and QEMU_LOCK_GUARD to simplify a few code paths now and in future. Signed-off-by: Jamie Iles --- cpus-common.c | 2 +- include/exec/cpu-common.h | 1 + linux-user/elfload.c | 12 ++-- migration/dirtyrate.c | 26 +- trace/control-target.c| 9 - 5 files changed, 25 insertions(+), 25 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] tests/9p: fix potential leak in v9fs_rreaddir()
Hi Christian !
On Sat, 29 Apr 2023 11:25:33 +0200
Christian Schoenebeck wrote:
> Free allocated directory entries in v9fs_rreaddir() if argument
> `entries` was passed as NULL, to avoid a memory leak. It is
> explicitly allowed by design for `entries` to be NULL. [1]
>
> [1] https://lore.kernel.org/all/1690923.g4PEXVpXuU@silver
>
> Reported-by: Coverity (CID 1487558)
> Signed-off-by: Christian Schoenebeck
> ---
Good catch Coverity ! :-)
Reviewed-by: Greg Kurz
I still have a suggestion. See below.
> tests/qtest/libqos/virtio-9p-client.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/tests/qtest/libqos/virtio-9p-client.c
> b/tests/qtest/libqos/virtio-9p-client.c
> index e4a368e036..b8adc8d4b9 100644
> --- a/tests/qtest/libqos/virtio-9p-client.c
> +++ b/tests/qtest/libqos/virtio-9p-client.c
> @@ -594,6 +594,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
> *nentries,
> {
> uint32_t local_count;
> struct V9fsDirent *e = NULL;
> +/* only used to avoid a leak if entries was NULL */
> +struct V9fsDirent *unused_entries = NULL;
> uint16_t slen;
> uint32_t n = 0;
>
> @@ -612,6 +614,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
> *nentries,
> e = g_new(struct V9fsDirent, 1);
> if (entries) {
> *entries = e;
> +} else {
> +unused_entries = e;
> }
> } else {
> e = e->next = g_new(struct V9fsDirent, 1);
This is always allocating and chaining a new entry even
though it isn't needed in the entries == NULL case.
> @@ -628,6 +632,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
> *nentries,
> *nentries = n;
> }
>
> +v9fs_free_dirents(unused_entries);
This is going to loop again on all entries to free them.
> v9fs_req_free(req);
> }
>
If this function is to be called one day with an enormous
number of entries and entries == NULL case, this might
not scale well.
What about only allocating a single entry in this case ?
E.g.
@@ -593,7 +593,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
*nentries,
struct V9fsDirent **entries)
{
uint32_t local_count;
-struct V9fsDirent *e = NULL;
+g_autofree struct V9fsDirent *e = NULL;
uint16_t slen;
uint32_t n = 0;
@@ -611,10 +611,12 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
*nentries,
if (!e) {
e = g_new(struct V9fsDirent, 1);
if (entries) {
-*entries = e;
+*entries = g_steal_pointer(e);
}
} else {
-e = e->next = g_new(struct V9fsDirent, 1);
+if (entries) {
+e = e->next = g_new(struct V9fsDirent, 1);
+}
}
e->next = NULL;
/* qid[13] offset[8] type[1] name[s] */
[PATCH] tests/9p: fix potential leak in v9fs_rreaddir()
Free allocated directory entries in v9fs_rreaddir() if argument
`entries` was passed as NULL, to avoid a memory leak. It is
explicitly allowed by design for `entries` to be NULL. [1]
[1] https://lore.kernel.org/all/1690923.g4PEXVpXuU@silver
Reported-by: Coverity (CID 1487558)
Signed-off-by: Christian Schoenebeck
---
tests/qtest/libqos/virtio-9p-client.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/tests/qtest/libqos/virtio-9p-client.c
b/tests/qtest/libqos/virtio-9p-client.c
index e4a368e036..b8adc8d4b9 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -594,6 +594,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
*nentries,
{
uint32_t local_count;
struct V9fsDirent *e = NULL;
+/* only used to avoid a leak if entries was NULL */
+struct V9fsDirent *unused_entries = NULL;
uint16_t slen;
uint32_t n = 0;
@@ -612,6 +614,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
*nentries,
e = g_new(struct V9fsDirent, 1);
if (entries) {
*entries = e;
+} else {
+unused_entries = e;
}
} else {
e = e->next = g_new(struct V9fsDirent, 1);
@@ -628,6 +632,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t
*nentries,
*nentries = n;
}
+v9fs_free_dirents(unused_entries);
v9fs_req_free(req);
}
--
2.30.2
Re: [PATCH] test-aio-multithread: do not use mb_read/mb_set for simple flags
On 4/28/23 12:12, Paolo Bonzini wrote: The remaining use of mb_read/mb_set is just to force a thread to exit eventually. It does not order two memory accesses and therefore can be just read/set. Signed-off-by: Paolo Bonzini --- tests/unit/test-aio-multithread.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] rcu: remove qatomic_mb_set, expand comments
On 4/28/23 11:50, Paolo Bonzini wrote: Signed-off-by: Paolo Bonzini --- include/qemu/rcu.h | 5 - util/rcu.c | 24 +++- 2 files changed, 15 insertions(+), 14 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v3 0/2] accel/tcg/tcg-accel-ops-rr: ensure fairness with icount
On 4/27/23 03:09, Jamie Iles wrote: From: Jamie Iles The round-robin scheduler will iterate over the CPU list with an assigned budget until the next timer expiry and may exit early because of a TB exit. This is fine under normal operation but with icount enabled and SMP it is possible for a CPU to be starved of run time and the system live-locks. For example, booting a riscv64 platform with '-icount shift=0,align=off,sleep=on -smp 2' we observe a livelock once the kernel has timers enabled and starts performing TLB shootdowns. In this case we have CPU 0 in M-mode with interrupts disabled sending an IPI to CPU 1. As we enter the TCG loop, we assign the icount budget to next timer interrupt to CPU 0 and begin executing where the guest is sat in a busy loop exhausting all of the budget before we try to execute CPU 1 which is the target of the IPI but CPU 1 is left with no budget with which to execute and the process repeats. We try here to add some fairness by splitting the budget across all of the CPUs on the thread fairly before entering each one. The CPU count is cached on CPU list generation ID to avoid iterating the list on each loop iteration. With this change it is possible to boot an SMP rv64 guest with icount enabled and no hangs. New in v3 (address feedback from Richard Henderson): - Additional patch to use QEMU_LOCK_GUARD with qemu_cpu_list_lock where appropriate - Move rr_cpu_count() call to be conditional on icount_enabled() - Initialize cpu_budget to 0 Jamie Iles (2): cpu: expose qemu_cpu_list_lock for lock-guard use accel/tcg/tcg-accel-ops-rr: ensure fairness with icount It appears as if one of these two patches causes a failure in replay, e.g. https://gitlab.com/rth7680/qemu/-/jobs/4200609234#L4162 Would you have a look, please? r~
Re: [PATCH v2] target/hexagon: fix = vs. == mishap
On Fri, 28 Apr 2023 at 21:45, Taylor Simpson wrote:
>
> From: Paolo Bonzini
>
> Changes in v2
> Fix yyassert's for sign and zero extends
>
> Coverity reports a parameter that is "set but never used". This is caused
> by an assignment operator being used instead of equality.
The commit message says it's fixing yyasserts, but the
new changed code doesn't seem to be fixing yyasserts?
> Co-authored-by: Taylor Simpson
> Signed-off-by: Paolo Bonzini
> Signed-off-by: Taylor Simpson
> ---
> target/hexagon/idef-parser/parser-helpers.c | 2 +-
> target/hexagon/idef-parser/idef-parser.y| 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/hexagon/idef-parser/parser-helpers.c
> b/target/hexagon/idef-parser/parser-helpers.c
> index 86511efb62..0a01ec39b7 100644
> --- a/target/hexagon/idef-parser/parser-helpers.c
> +++ b/target/hexagon/idef-parser/parser-helpers.c
> @@ -1123,7 +1123,7 @@ HexValue gen_extend_op(Context *c,
> HexValue *value,
> HexSignedness signedness)
> {
> -unsigned bit_width = (dst_width = 64) ? 64 : 32;
> +unsigned bit_width = (dst_width == 64) ? 64 : 32;
> HexValue value_m = *value;
> HexValue src_width_m = *src_width;
>
> diff --git a/target/hexagon/idef-parser/idef-parser.y
> b/target/hexagon/idef-parser/idef-parser.y
> index 5444fd4749..2561f0ebb0 100644
> --- a/target/hexagon/idef-parser/idef-parser.y
> +++ b/target/hexagon/idef-parser/idef-parser.y
> @@ -685,7 +685,7 @@ rvalue : FAIL
> yyassert(c, &@1, $5.type == IMMEDIATE &&
>$5.imm.type == VALUE,
>"SXT expects immediate values\n");
> - $$ = gen_extend_op(c, &@1, &$3, $5.imm.value, &$7, SIGNED);
> + $$ = gen_extend_op(c, &@1, &$3, 64, &$7, SIGNED);
> }
> | ZXT '(' rvalue ',' IMM ',' rvalue ')'
> {
> @@ -693,7 +693,7 @@ rvalue : FAIL
> yyassert(c, &@1, $5.type == IMMEDIATE &&
>$5.imm.type == VALUE,
>"ZXT expects immediate values\n");
> - $$ = gen_extend_op(c, &@1, &$3, $5.imm.value, &$7, UNSIGNED);
> + $$ = gen_extend_op(c, &@1, &$3, 64, &$7, UNSIGNED);
> }
> | '(' rvalue ')'
> {
> --
> 2.25.1
thanks
-- PMM
Re: [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS
On 2023/4/29 16:54, Richard Henderson wrote:
On 4/28/23 17:52, Mayuresh Chitale wrote:
When misa.F is 0 tb->flags.FS field is unused and can be used to save
the current state of smstateen0.FCSR check which is needed by the
floating point translation routines.
Signed-off-by: Mayuresh Chitale
---
target/riscv/cpu_helper.c | 9 +
target/riscv/translate.c | 12 +++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b68dcfe7b6..126ac221a0 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -119,6 +119,15 @@ void cpu_get_tb_cpu_state(CPURISCVState *env,
target_ulong *pc,
vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
}
+ /*
+ * If misa.F is 0 then the FS field of the tb->flags can be used
to pass
+ * the current state of the smstateen.FCSR bit which must be
checked for
+ * in the floating point translation routines.
+ */
+ if (!riscv_has_ext(env, RVF)) {
+ fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) ==
RISCV_EXCP_NONE);
+ }
You have misunderstood my suggestion:
/* With Zfinx, floating point is enabled/disabled by Smstateen. */
if (!riscv_has_ext(env, RVF)) {
fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR)
? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED);
}
+ bool smstateen_fcsr_ok;
Not needed.
- ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+ if (has_ext(ctx, RVF)) {
+ ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+ } else {
+ ctx->mstatus_fs = 0;
+ }
Not needed.
In patch 3, which should be merged with this, there are no changes to
REQUIRE_ZFINX_OR_F, no additional smstateen_fcsr_check, and
REQUIRE_FPU reduces to
#define REQUIRE_FPU do { \
if (ctx->mstatus_fs == EXT_STATUS_DISABLED) { \
return false; \
} \
} while (0)
This makes the DisasContext version of fs be the single gate for
floating point.
No extra checks required.
Yeah. It's better to merge with REQUIRE_FPU.
However, virtual instruction exception will be triggered in VS/VU mode
if smstateen check fails, which is different from normal REQUIRE_FPU.
So, we may need to modify REQUIRE_FPU to distinguish them:
#define REQUIRE_FPU do { \
if (ctx->mstatus_fs == EXT_STATUS_DISABLED) { \
ctx->virt_inst_excp = ctx->virt_enabled &&
ctx->cfg_ptr->ext_zfinx; \
return false; \
} \
} while (0)
Regards,
Weiwei Li
r~
Re: [PULL 00/17] QAPI patches patches for 2023-04-28
On 4/28/23 11:28, Markus Armbruster wrote: The following changes since commit cc5ee50fff9dbac0aac32cd892a7163c7babcca1: Merge tag 'pull-testing-docs-270423-1' ofhttps://gitlab.com/stsquad/qemu into staging (2023-04-27 16:46:17 +0100) are available in the Git repository at: https://repo.or.cz/qemu/armbru.git tags/pull-qapi-2023-04-28 for you to fetch changes up to e2e9e567f0e23971cac35ba1dee7edb51085b5f7: docs/devel/qapi-code-gen: Describe some doc markup pitfalls (2023-04-28 11:48:34 +0200) QAPI patches patches for 2023-04-28 Markus Armbruster (17): qga/qapi-schema: Tidy up documentation of guest-fsfreeze-status qga/qapi-schema: Fix a misspelled reference qapi: Fix misspelled references qapi: Fix up references to long gone error classes qapi/block-core: Clean up after removal of dirty bitmap @status qapi: @foo should be used to reference, not ``foo`` qapi: Tidy up examples qapi: Delete largely misleading "Stability Considerations" qapi: Fix bullet list markup in documentation qapi: Fix unintended definition lists in documentation qga/qapi-schema: Fix member documentation markup qapi: Fix argument documentation markup qapi: Replace ad hoc "since" documentation by member documentation qapi: Fix misspelled section tags in doc comments qapi: Format since information the conventional way: (since X.Y) qapi storage-daemon/qapi: Fix documentation section structure docs/devel/qapi-code-gen: Describe some doc markup pitfalls Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate. r~
Re: [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS
On 4/28/23 17:52, Mayuresh Chitale wrote:
When misa.F is 0 tb->flags.FS field is unused and can be used to save
the current state of smstateen0.FCSR check which is needed by the
floating point translation routines.
Signed-off-by: Mayuresh Chitale
---
target/riscv/cpu_helper.c | 9 +
target/riscv/translate.c | 12 +++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b68dcfe7b6..126ac221a0 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -119,6 +119,15 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong
*pc,
vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
}
+/*
+ * If misa.F is 0 then the FS field of the tb->flags can be used to pass
+ * the current state of the smstateen.FCSR bit which must be checked for
+ * in the floating point translation routines.
+ */
+if (!riscv_has_ext(env, RVF)) {
+fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE);
+}
You have misunderstood my suggestion:
/* With Zfinx, floating point is enabled/disabled by Smstateen. */
if (!riscv_has_ext(env, RVF)) {
fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR)
? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED);
}
+bool smstateen_fcsr_ok;
Not needed.
-ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+if (has_ext(ctx, RVF)) {
+ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+} else {
+ctx->mstatus_fs = 0;
+}
Not needed.
In patch 3, which should be merged with this, there are no changes to REQUIRE_ZFINX_OR_F,
no additional smstateen_fcsr_check, and REQUIRE_FPU reduces to
#define REQUIRE_FPU do { \
if (ctx->mstatus_fs == EXT_STATUS_DISABLED) { \
return false; \
} \
} while (0)
This makes the DisasContext version of fs be the single gate for floating point.
No extra checks required.
r~
