[PATCH v3] virtio: increase virtqueue size for virtio-scsi and virtio-blk
The goal is to reduce the amount of requests issued by a guest on
1M reads/writes. This rises the performance up to 4% on that kind of
disk access pattern.
The maximum chunk size to be used for the guest disk accessing is
limited with seg_max parameter, which represents the max amount of
pices in the scatter-geather list in one guest disk request.
Since seg_max is virqueue_size dependent, increasing the virtqueue
size increases seg_max, which, in turn, increases the maximum size
of data to be read/write from a guest disk.
More details in the original problem statment:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
Suggested-by: Denis V. Lunev
Signed-off-by: Denis Plotnikov
---
v3:
* typos fixed
v2:
* seg_max default value changing removed
---
hw/block/virtio-blk.c | 2 +-
hw/core/machine.c | 2 ++
hw/scsi/virtio-scsi.c | 2 +-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..142863a3b2 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
-DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2501b540ec..3427d6cf4c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@
#include "hw/mem/nvdimm.h"
GlobalProperty hw_compat_4_2[] = {
+{ "virtio-blk-device", "queue-size", "128"},
+{ "virtio-scsi-device", "virtqueue_size", "128"},
{ "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
{ "virtio-blk-device", "seg-max-adjust", "off"},
{ "virtio-scsi-device", "seg_max_adjust", "off"},
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..472bbd233b 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev,
Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
1),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
- parent_obj.conf.virtqueue_size, 128),
+ parent_obj.conf.virtqueue_size, 256),
DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
parent_obj.conf.seg_max_adjust, true),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
--
2.17.0
RE: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Thursday, February 13, 2020 6:17 PM
> To: miaoyubo
> Cc: peter.mayd...@linaro.org; shannon.zha...@gmail.com; Xiexiangyou
> ; imamm...@redhat.com; qemu-
> de...@nongnu.org
> Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie
> under arm.
>
> On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > From: miaoyubo
> >
> > Since devices could not directly plugged into pxb-pcie,
>
> Hmm is this different from the root port? intergrated devices do exist for
> that actually.
>
Thanks for replying
The pxb-pcie is like a host bridge,
you have to plug pcie-root-port or pci-bridge so devices could be plugged
> > under arm,
>
> how is arm special?
>
Cureently, if u define a pxb-pcie device, one pcie-root-port or pci-bridge or
something
else have to be defined also, The patch just auto generate pcie-root-port for
arm to
avoid affect other architecture
> > one
> > pcie-root port is plugged into pxb-pcie. Due to the bus for each
> > pxb-pcie is defined as 2 in acpi dsdt tables(one for pxb-pcie, one for
> > pcie-root-port), only one device could be plugged into one pxb-pcie.
>
> So why can't we have users specify any number of root ports using -device?
> then make acpi tables match the # of ports created?
>
>
Users could specify multiply pxb-devices, it is supported.
But only one device could be plugged for each pxb-pcie, it is the same with
pxb-pci for piix.
> >
> > Signed-off-by: miaoyubo
> > ---
> > hw/pci-bridge/pci_expander_bridge.c | 9 +
> > include/hw/pci/pcie_port.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c
> > b/hw/pci-bridge/pci_expander_bridge.c
> > index 47aaaf8fd1..3d896dd452 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -15,6 +15,7 @@
> > #include "hw/pci/pci.h"
> > #include "hw/pci/pci_bus.h"
> > #include "hw/pci/pci_host.h"
> > +#include "hw/pci/pcie_port.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/pci/pci_bridge.h"
> > #include "qemu/range.h"
> > @@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice
> > *dev, bool pcie, Error **errp)
> >
> > ds = qdev_create(NULL, TYPE_PXB_HOST);
> > if (pcie) {
> > +#ifdef __aarch64__
> > +bus = pci_root_bus_new(ds, "pxb-pcie-internal",
> > + NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> > +bds = qdev_create(BUS(bus), "pcie-root-port");
> > +bds->id = dev_name;
> > +qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS,
> > +pxb->bus_nr); #else
> > bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0,
> > TYPE_PXB_PCIE_BUS);
> > +#endif
>
> What does all this have to do with building on aarch64?
>
Based on the comments, this patch would be abandoned in patch V2,
PXB-PCIE would also be useful but pcie-root-port or pci-bridge have to Be
defined by user.
> > } else {
> > bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0,
> TYPE_PXB_BUS);
> > bds = qdev_create(BUS(bus), "pci-bridge"); diff --git
> > a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h index
> > 4b3d254b08..b41d473220 100644
> > --- a/include/hw/pci/pcie_port.h
> > +++ b/include/hw/pci/pcie_port.h
> > @@ -64,6 +64,7 @@ int pcie_chassis_add_slot(struct PCIESlot *slot);
> > void pcie_chassis_del_slot(PCIESlot *s);
> >
> > #define TYPE_PCIE_ROOT_PORT "pcie-root-port-base"
> > +#define PCIE_ROOT_PORT_PROP_CHASSIS "chassis"
>
> If you are going to do this, replace other instances of "chassis"
> with the macro.
>
Thanks for your replay, this patch would be abandoned.
> > #define PCIE_ROOT_PORT_CLASS(klass) \
> > OBJECT_CLASS_CHECK(PCIERootPortClass, (klass),
> > TYPE_PCIE_ROOT_PORT) #define PCIE_ROOT_PORT_GET_CLASS(obj) \
> > --
> > 2.19.1
> >
Regards,
Miao
RE: [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support PXB-PCIE
> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Thursday, February 13, 2020 6:23 PM
> To: miaoyubo
> Cc: peter.mayd...@linaro.org; shannon.zha...@gmail.com; Xiexiangyou
> ; imamm...@redhat.com; qemu-
> de...@nongnu.org
> Subject: Re: [RFC 1/2] arm: acpi: pci-expender-bus: Make arm to support
> PXB-PCIE
>
> On Thu, Feb 13, 2020 at 03:49:51PM +0800, Yubo Miao wrote:
> > From: miaoyubo
> >
> > Currently virt machine is not supported by pxb-pcie, and only one main
> > host bridge described in ACPI tables.
> > Under this circumstance, different io numas for differnt devices is
> > not possible, in order to present io numas to the guest, especially
> > for host pssthrough devices. PXB-PCIE is supproted by arm and certain
> > resource is allocated for each pxb-pcie in acpi table.
> >
> > Signed-off-by: miaoyubo
> > ---
> > hw/arm/virt-acpi-build.c | 234
> +--
> > hw/pci-host/gpex.c | 4 +
> > include/hw/arm/virt.h| 1 +
> > 3 files changed, 228 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > bd5f771e9b..2e449d0098 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -49,6 +49,8 @@
> > #include "kvm_arm.h"
> > #include "migration/vmstate.h"
> >
> > +#include "hw/arm/virt.h"
> > +#include "hw/pci/pci_bus.h"
> > #define ARM_SPI_BASE 32
> >
> > +/*
> > + * PCI Firmware Specification 3.0
> > + * 4.6.1. _DSM for PCI Express Slot Information
> > + * The UUID in _DSM in this context is
> > + * {E5C937D0-3553-4D7A-9117-EA4D19C3434D}
> > + */
> > +UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> > +ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> > +ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> > +uint8_t byte_list[1] = {1};
> > +buf = aml_buffer(1, byte_list);
> > +aml_append(ifctx1, aml_return(buf));
> > +aml_append(ifctx, ifctx1);
> > +aml_append(method, ifctx);
> > +
> > +byte_list[0] = 0;
> > +buf = aml_buffer(1, byte_list);
> > +aml_append(method, aml_return(buf));
> > +aml_append(dev, method);
> > +
> > +Aml *dev_rp0 = aml_device("%s", "RP0");
> > +aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> > +aml_append(dev, dev_rp0);
> > +
> > +aml_append(scope, dev);
> > +
> > +}
> > +}
>
>
> There's a bunch of code duplicated here. Please refactor creating APIs for
> reused code.
>
Thanks for your reply, this would be done by patch V2.
> >
> > -Aml *dev = aml_device("%s", "PCI0");
> > +dev = aml_device("%s", "PCI0");
> > aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> > aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> > aml_append(dev, aml_name_decl("_SEG", aml_int(0))); @@ -219,16
> > +428,18 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap,
> > Aml *rbuf = aml_resource_template();
> > aml_append(rbuf,
> > aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE,
> > -0x, 0x, nr_pcie_buses - 1, 0x,
> > -nr_pcie_buses));
> > +0x, 0x, root_bus_limit, 0x,
> > +root_bus_limit + 1));
> > aml_append(rbuf,
> > aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> AML_MAX_FIXED,
> > AML_NON_CACHEABLE, AML_READ_WRITE, 0x,
> base_mmio,
> > - base_mmio + size_mmio - 1, 0x, size_mmio));
> > + base_mmio + size_mmio - 1 - size_addr * count,
> > + 0x, size_mmio - size_addr * count));
> > aml_append(rbuf,
> > aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE,
> > - AML_ENTIRE_RANGE, 0x, 0x, size_pio - 1,
> > base_pio,
> > - size_pio));
> > + AML_ENTIRE_RANGE, 0x, 0x,
> > + size_pio / 2 - 1 - size_io * count, base_pio,
> > + size_pio / 2 - size_io * count));
> >
> > if (use_highmem) {
> > hwaddr base_mmio_high = memmap[VIRT_HIGH_PCIE_MMIO].base;
> @@
> > -238,8 +449,9 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> MemMapEntry *memmap,
> > aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
> AML_MAX_FIXED,
> > AML_NON_CACHEABLE, AML_READ_WRITE, 0x,
> > base_mmio_high,
> > - base_mmio_high + size_mmio_high - 1, 0x,
> > - size_mmio_high));
> > + base_mmio_high +
RE: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie under arm.
> -Original Message-
> From: Daniel P. Berrangé [mailto:berra...@redhat.com]
> Sent: Thursday, February 13, 2020 9:52 PM
> To: miaoyubo
> Cc: peter.mayd...@linaro.org; shannon.zha...@gmail.com;
> imamm...@redhat.com; qemu-devel@nongnu.org; Xiexiangyou
> ; m...@redhat.com
> Subject: Re: [RFC 2/2] pci-expender-bus:Add pcie-root-port to pxb-pcie
> under arm.
>
> On Thu, Feb 13, 2020 at 03:49:52PM +0800, Yubo Miao wrote:
> > From: miaoyubo
> >
> > Since devices could not directly plugged into pxb-pcie, under arm, one
> > pcie-root port is plugged into pxb-pcie. Due to the bus for each
> > pxb-pcie is defined as 2 in acpi dsdt tables(one for pxb-pcie, one for
> > pcie-root-port), only one device could be plugged into one pxb-pcie.
>
> What is the cause of this arm specific requirement for pxb-pcie and more
> importantly can be fix it so that we don't need this patch ?
> I think it is highly undesirable to have such a per-arch difference in
> configuration of the pxb-pcie device. It means any mgmt app which already
> supports pxb-pcie will be broken and need to special case arm.
>
Thanks for your reply, Without this patch, the pxb-pcie is also useable,
however, one extra pcie-root-port or pci-bridge or something else need
to be defined by mgmt. app. This patch will could be abandoned.
> >
> > Signed-off-by: miaoyubo
> > ---
> > hw/pci-bridge/pci_expander_bridge.c | 9 +
> > include/hw/pci/pcie_port.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c
> > b/hw/pci-bridge/pci_expander_bridge.c
> > index 47aaaf8fd1..3d896dd452 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -15,6 +15,7 @@
> > #include "hw/pci/pci.h"
> > #include "hw/pci/pci_bus.h"
> > #include "hw/pci/pci_host.h"
> > +#include "hw/pci/pcie_port.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/pci/pci_bridge.h"
> > #include "qemu/range.h"
> > @@ -233,7 +234,15 @@ static void pxb_dev_realize_common(PCIDevice
> > *dev, bool pcie, Error **errp)
> >
> > ds = qdev_create(NULL, TYPE_PXB_HOST);
> > if (pcie) {
> > +#ifdef __aarch64__
> > +bus = pci_root_bus_new(ds, "pxb-pcie-internal",
> > + NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> > +bds = qdev_create(BUS(bus), "pcie-root-port");
> > +bds->id = dev_name;
> > +qdev_prop_set_uint8(bds, PCIE_ROOT_PORT_PROP_CHASSIS,
> > +pxb->bus_nr); #else
> > bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0,
> > TYPE_PXB_PCIE_BUS);
> > +#endif
> > } else {
> > bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0,
> TYPE_PXB_BUS);
> > bds = qdev_create(BUS(bus), "pci-bridge"); diff --git
> > a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h index
> > 4b3d254b08..b41d473220 100644
> > --- a/include/hw/pci/pcie_port.h
> > +++ b/include/hw/pci/pcie_port.h
> > @@ -64,6 +64,7 @@ int pcie_chassis_add_slot(struct PCIESlot *slot);
> > void pcie_chassis_del_slot(PCIESlot *s);
> >
> > #define TYPE_PCIE_ROOT_PORT "pcie-root-port-base"
> > +#define PCIE_ROOT_PORT_PROP_CHASSIS "chassis"
> > #define PCIE_ROOT_PORT_CLASS(klass) \
> > OBJECT_CLASS_CHECK(PCIERootPortClass, (klass),
> > TYPE_PCIE_ROOT_PORT) #define PCIE_ROOT_PORT_GET_CLASS(obj) \
> > --
> > 2.19.1
> >
> >
> >
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
Regards,
Miao
[PATCH 3/3] hw/riscv/spike: Allow more than one CPUs
Currently, the upstream Spike ISA simulator allows more than
one CPUs so we update QEMU Spike machine on similar lines to
allow more than one CPUs.
The maximum number of CPUs for QEMU Spike machine is kept
same as QEMU Virt machine.
Signed-off-by: Anup Patel
---
hw/riscv/spike.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 060a86f922..1eac0d9a83 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -469,7 +469,7 @@ static void spike_machine_init(MachineClass *mc)
{
mc->desc = "RISC-V Spike Board";
mc->init = spike_board_init;
-mc->max_cpus = 1;
+mc->max_cpus = 8;
mc->is_default = 1;
mc->default_cpu_type = SPIKE_V1_10_0_CPU;
}
--
2.17.1
[PATCH 1/3] hw/riscv: Add optional symbol callback ptr to riscv_load_firmware()
This patch adds an optional function pointer, "sym_cb", to
riscv_load_firmware() which provides the possibility to access
the symbol table during kernel loading.
The pointer is ignored, if supplied with flat (non-elf) firmware image.
The Spike board requires it locate the HTIF symbols from firmware ELF
passed via "-bios" option.
Signed-off-by: Anup Patel
---
hw/riscv/boot.c | 13 -
hw/riscv/sifive_u.c | 2 +-
hw/riscv/virt.c | 2 +-
include/hw/riscv/boot.h | 6 --
4 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 027303d2a3..7ec94dc701 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -36,7 +36,8 @@
void riscv_find_and_load_firmware(MachineState *machine,
const char *default_machine_firmware,
- hwaddr firmware_load_addr)
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb)
{
char *firmware_filename = NULL;
@@ -76,7 +77,7 @@ void riscv_find_and_load_firmware(MachineState *machine,
if (firmware_filename) {
/* If not "none" load the firmware */
-riscv_load_firmware(firmware_filename, firmware_load_addr);
+riscv_load_firmware(firmware_filename, firmware_load_addr, sym_cb);
g_free(firmware_filename);
}
}
@@ -96,12 +97,14 @@ char *riscv_find_firmware(const char *firmware_filename)
}
target_ulong riscv_load_firmware(const char *firmware_filename,
- hwaddr firmware_load_addr)
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb)
{
uint64_t firmware_entry, firmware_start, firmware_end;
-if (load_elf(firmware_filename, NULL, NULL, NULL, _entry,
- _start, _end, 0, EM_RISCV, 1, 0) > 0) {
+if (load_elf_ram_sym(firmware_filename, NULL, NULL, NULL,
+ _entry, _start, _end, 0,
+ EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) {
return firmware_entry;
}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 0140e95732..0c84215f42 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -341,7 +341,7 @@ static void riscv_sifive_u_init(MachineState *machine)
create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
riscv_find_and_load_firmware(machine, BIOS_FILENAME,
- memmap[SIFIVE_U_DRAM].base);
+ memmap[SIFIVE_U_DRAM].base, NULL);
if (machine->kernel_filename) {
uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index c44b865959..90a5bfef63 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -476,7 +476,7 @@ static void riscv_virt_board_init(MachineState *machine)
mask_rom);
riscv_find_and_load_firmware(machine, BIOS_FILENAME,
- memmap[VIRT_DRAM].base);
+ memmap[VIRT_DRAM].base, NULL);
if (machine->kernel_filename) {
uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index df80051fbc..474a940ad5 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -24,10 +24,12 @@
void riscv_find_and_load_firmware(MachineState *machine,
const char *default_machine_firmware,
- hwaddr firmware_load_addr);
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb);
char *riscv_find_firmware(const char *firmware_filename);
target_ulong riscv_load_firmware(const char *firmware_filename,
- hwaddr firmware_load_addr);
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb);
target_ulong riscv_load_kernel(const char *kernel_filename,
symbol_fn_t sym_cb);
hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
--
2.17.1
[PATCH 0/3] RISC-V Spike machine improvements
This series improves QEMU Spike machine to: 1. Allow loading OpenBI firmware using -bios option 2. Allow more than one CPUs Anup Patel (3): hw/riscv: Add optional symbol callback ptr to riscv_load_firmware() hw/riscv/spike: Allow loading firmware separately using -bios option hw/riscv/spike: Allow more than one CPUs hw/riscv/boot.c | 13 - hw/riscv/sifive_u.c | 2 +- hw/riscv/spike.c| 26 -- hw/riscv/virt.c | 2 +- include/hw/riscv/boot.h | 6 -- 5 files changed, 38 insertions(+), 11 deletions(-) -- 2.17.1
[PATCH 2/3] hw/riscv/spike: Allow loading firmware separately using -bios option
This patch extends Spike machine support to allow loading OpenSBI
firmware (fw_jump.elf) separately using -bios option.
Signed-off-by: Anup Patel
---
hw/riscv/spike.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 8823681783..060a86f922 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -45,6 +45,12 @@
#include
+#if defined(TARGET_RISCV32)
+# define BIOS_FILENAME "opensbi-riscv32-spike-fw_jump.elf"
+#else
+# define BIOS_FILENAME "opensbi-riscv64-spike-fw_jump.elf"
+#endif
+
static const struct MemmapEntry {
hwaddr base;
hwaddr size;
@@ -183,8 +189,24 @@ static void spike_board_init(MachineState *machine)
memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
mask_rom);
+riscv_find_and_load_firmware(machine, BIOS_FILENAME,
+ memmap[SPIKE_DRAM].base,
+ htif_symbol_callback);
+
if (machine->kernel_filename) {
-riscv_load_kernel(machine->kernel_filename, htif_symbol_callback);
+uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
+ htif_symbol_callback);
+
+if (machine->initrd_filename) {
+hwaddr start;
+hwaddr end = riscv_load_initrd(machine->initrd_filename,
+ machine->ram_size, kernel_entry,
+ );
+qemu_fdt_setprop_cell(s->fdt, "/chosen",
+ "linux,initrd-start", start);
+qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
+ end);
+}
}
/* reset vector */
--
2.17.1
Re: [PATCH v2 18/30] qapi: Delete all the "foo: dropped in n.n" notes
Peter Maydell writes:
> A handful of QAPI doc comments include lines like
> "ppcemb: dropped in 3.1". The doc comment parser will just
> put these into whatever the preceding section was; sometimes
> that's "Notes", and sometimes it's some random other section,
> as with "NetClientDriver" where the "'dump': dropped in 2.12"
> line ends up in the "Since:" section.
>
> This tends to render wrongly, more so in the upcoming rST
> generator, but sometimes even in the texinfo, as in the case
> of QKeyCode:
>ac_bookmarks
>since 2.10 altgr, altgr_r: dropped in 2.10
>
> We now have a better place to tell users about deprecated
> and deleted functionality -- qemu-deprecated.texi.
> So just remove all these "dropped in" remarks entirely.
>
> Signed-off-by: Peter Maydell
> ---
> Perhaps qemu-deprecated.texi should be updated -- Markus
> said he'd look into that. So this patch is to some extent
> a placeholder to get these broken bits of doc comment out
> of the way.
The appropriate place is appendix "Recently removed features", which
appeared in commit 3264ffced3 "dirty-bitmaps: remove deprecated autoload
parameter", v4.2.0. We did not document any prior removals then.
Perhaps we should systematically document all removals since v4.1.0. I
can look into that.
I'm not sure documenting older removals now is worth our while. If you
think it is, let me know.
All the 'dropped in' notes removed in this patch are older. Nothing to
do for qemu-deprecated.texi unless we choose to systematically document
older removals.
> ---
> qapi/machine.json | 2 --
> qapi/net.json | 4
> qapi/ui.json | 1 -
> 3 files changed, 7 deletions(-)
>
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 704b2b0fe31..6c11e3cf3a4 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -20,8 +20,6 @@
> #prefix to produce the corresponding QEMU executable name. This
> #is true even for "qemu-system-x86_64".
> #
> -# ppcemb: dropped in 3.1
> -#
> # Since: 3.0
> ##
> { 'enum' : 'SysEmuTarget',
This is SysEmuTarget. Visible in QMP query-target and query-cpus-fast.
> diff --git a/qapi/net.json b/qapi/net.json
> index 80dcf0df06e..1cb9a7d782b 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -446,8 +446,6 @@
> # Available netdev drivers.
> #
> # Since: 2.7
> -#
> -# 'dump': dropped in 2.12
> ##
> { 'enum': 'NetClientDriver',
>'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
This is enum NetClientDriver. Visible in QMP netdev_add and -netdev.
> @@ -493,8 +491,6 @@
> # @opts: device type specific properties (legacy)
> #
> # Since: 1.2
> -#
> -# 'vlan': dropped in 3.0
> ##
> { 'struct': 'NetLegacy',
>'data': {
This is struct NetLegacy. Visible in -net, I think.
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 89126da395b..e16e98a060d 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -779,7 +779,6 @@
> # @ac_forward: since 2.10
> # @ac_refresh: since 2.10
> # @ac_bookmarks: since 2.10
> -# altgr, altgr_r: dropped in 2.10
> #
> # @muhenkan: since 2.12
> # @katakanahiragana: since 2.12
This is enum QKeyCode. Visible in QMP send-key arguments.
Re: [PATCH v2 01/30] configure: Allow user to specify sphinx-build binary
Does not work out of the box on my Fedora 30 build host, where
sphinx-build gives me sphinx-build-2. I have to specify
--sphinx-build=/usr/bin/sphinx-build-3 to unbreak it. Which of course
breaks things when I try to build anything before this commit
The appended patch makes it work out of the box. Please consider
squashing it in.
diff --git a/configure b/configure
index 14172909f0..a9d175c400 100755
--- a/configure
+++ b/configure
@@ -584,7 +584,6 @@ query_pkg_config() {
}
pkg_config=query_pkg_config
sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
-sphinx_build=sphinx-build
# If the user hasn't specified ARFLAGS, default to 'rv', just as make does.
ARFLAGS="${ARFLAGS-rv}"
@@ -903,6 +902,7 @@ fi
: ${make=${MAKE-make}}
: ${install=${INSTALL-install}}
+
# We prefer python 3.x. A bare 'python' is traditionally
# python 2.x, but some distros have it as python 3.x, so
# we check that too
@@ -915,6 +915,19 @@ do
break
fi
done
+
+set -x
+sphinx_build=
+for binary in sphinx-build-3 sphinx-build
+do
+if has "$binary"
+then
+sphinx_build=$(command -v "$binary")
+break
+fi
+done
+set +x
+
: ${smbd=${SMBD-/usr/sbin/smbd}}
# Default objcc to clang if available, otherwise use CC
@@ -4803,7 +4816,7 @@ has_sphinx_build() {
# sphinx-build doesn't exist at all or if it is too old.
mkdir -p "$TMPDIR1/sphinx"
touch "$TMPDIR1/sphinx/index.rst"
-$sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx"
"$TMPDIR1/sphinx/out" >/dev/null 2>&1
+"$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx"
"$TMPDIR1/sphinx/out" >/dev/null 2>&1
}
# Check if tools are available to build documentation.
Re: [RESEND RFC PATCH v2 2/2] target/arm: Support NMI injection
On 2/13/20 10:11 PM, Peter Maydell wrote: On Wed, 5 Feb 2020 at 11:06, Gavin Shan wrote: This supports QMP/HMP "nmi" command by injecting SError interrupt to guest, which is expected to crash with that. Currently, It's supported on two CPU models: "host" and "max". Signed-off-by: Gavin Shan --- hw/arm/virt.c | 18 target/arm/cpu-qom.h | 1 + target/arm/cpu.c | 48 ++ target/arm/cpu64.c | 25 ++ target/arm/internals.h | 8 +++ 5 files changed, 96 insertions(+), 4 deletions(-) A few quick general notes: (1) as I mentioned on the cover letter, the mechanism for injecting an SError/async external abort into the CPU should be a qemu_irq line, like FIQ/IRQ, not a special-purpose method on the CPU object. (2) for function naming, there's a dividing line between: * code that implements the (unfortunately x86-centric) monitor command named "nmi"; these functions can have names with 'nmi' in them * code which implements the actual mechanism of 'deliver an SError to the CPU'; these functions should not have 'nmi' in the name or mention nmi, because nmi is not a concept in the Arm architecture (3) Before we expose 'nmi' to users as something that delivers an SError, we need to think about the interactions with RAS, because currently we also use SError to say "there was an error in the host memory you're using", and we might in future want to use SError for proper emulated RAS. We don't want to paint ourselves into a corner by grabbing SError exclusively for 'nmi'. Hi Peter, Thanks for the nice details. I just posted v3 to address (1) and (2). For (3), I'm not sure. It seems we need some registers to record the details on why the SError is raised. For ARMv8 with RAS extension is supported, VSESR_EL2 can be used. Otherwise, we probably need some space in ESR_EL1. For aarch32, DFSR might be the alternative. With more details about the cause, the "NMI" and other errors can be classified. May I ask if the SError is going to be triggered by simulated device or real one, or both in future? If it's corresponding to host memory error, it seems it should be triggered by a real hardware. In this case, the error should be intercepted and then delivered to guest. I need more details how the SError will be used for RAS. Thanks, Gavin
[PATCH 3/3] arm: allwinner: Wire up USB EHCI
Initialize EHCI controllers on Allwinner A10. With this patch applied,
USB EHCI ports are discovered when booting the cubieboard machine with
a recent Linux kernel.
ehci-platform 1c14000.usb: EHCI Host Controller
ehci-platform 1c14000.usb: new USB bus registered, assigned bus number 1
ehci-platform 1c14000.usb: irq 26, io mem 0x01c14000
ehci-platform 1c14000.usb: USB 2.0 started, EHCI 1.00
ehci-platform 1c1c000.usb: EHCI Host Controller
ehci-platform 1c1c000.usb: new USB bus registered, assigned bus number 2
ehci-platform 1c1c000.usb: irq 31, io mem 0x01c1c000
ehci-platform 1c1c000.usb: USB 2.0 started, EHCI 1.00
Signed-off-by: Guenter Roeck
---
hw/arm/allwinner-a10.c | 17 +
include/hw/arm/allwinner-a10.h | 2 ++
2 files changed, 19 insertions(+)
diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 818145428c..f8b963b5c3 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -32,6 +32,7 @@
#define AW_A10_PIT_REG_BASE 0x01c20c00
#define AW_A10_UART0_REG_BASE 0x01c28000
#define AW_A10_EMAC_BASE0x01c0b000
+#define AW_A10_EHCI_BASE0x01c14000
#define AW_A10_OHCI_BASE0x01c14400
#define AW_A10_SATA_BASE0x01c18000
@@ -64,6 +65,10 @@ static void aw_a10_init(Object *obj)
sysbus_init_child_obj(obj, "ohci[*]", OBJECT(>ohci[i]),
sizeof(s->ohci[i]), TYPE_SYSBUS_OHCI);
}
+for (i = 0; i < ARRAY_SIZE(s->ehci); i++) {
+sysbus_init_child_obj(obj, "ehci[*]", OBJECT(>ehci[i]),
+ sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
+}
}
}
@@ -162,6 +167,18 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(>ohci[i]), 0,
qdev_get_gpio_in(dev, 64 + i));
}
+for (i = 0; i < ARRAY_SIZE(s->ehci); i++) {
+object_property_set_bool(OBJECT(>ehci[i]), true, "realized",
+ );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>ehci[i]), 0,
+AW_A10_EHCI_BASE + i * 0x8000);
+sysbus_connect_irq(SYS_BUS_DEVICE(>ehci[i]), 0,
+ qdev_get_gpio_in(dev, 39 + i));
+}
}
}
diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
index 4864adbec3..17e1ee92e2 100644
--- a/include/hw/arm/allwinner-a10.h
+++ b/include/hw/arm/allwinner-a10.h
@@ -10,6 +10,7 @@
#include "hw/net/allwinner_emac.h"
#include "hw/ide/ahci.h"
#include "hw/usb/hcd-ohci.h"
+#include "hw/usb/hcd-ehci.h"
#include "target/arm/cpu.h"
@@ -31,6 +32,7 @@ typedef struct AwA10State {
AllwinnerAHCIState sata;
MemoryRegion sram_a;
OHCISysBusState ohci[2];
+EHCISysBusState ehci[2];
} AwA10State;
#endif
--
2.17.1
[PATCH 1/3] hw: usb: hcd-ohci: Move OHCISysBusState and TYPE_SYSBUS_OHCI to include file
We need to be able to use OHCISysBusState outside hcd-ohci.c, so move it
to its include file.
Signed-off-by: Guenter Roeck
---
hw/usb/hcd-ohci.c | 15 ---
hw/usb/hcd-ohci.h | 16
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 8a94bd004a..1e6e85e86a 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1870,21 +1870,6 @@ void ohci_sysbus_die(struct OHCIState *ohci)
ohci_bus_stop(ohci);
}
-#define TYPE_SYSBUS_OHCI "sysbus-ohci"
-#define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
-
-typedef struct {
-/*< private >*/
-SysBusDevice parent_obj;
-/*< public >*/
-
-OHCIState ohci;
-char *masterbus;
-uint32_t num_ports;
-uint32_t firstport;
-dma_addr_t dma_offset;
-} OHCISysBusState;
-
static void ohci_realize_pxa(DeviceState *dev, Error **errp)
{
OHCISysBusState *s = SYSBUS_OHCI(dev);
diff --git a/hw/usb/hcd-ohci.h b/hw/usb/hcd-ohci.h
index 16e3f1e13a..5c8819aedf 100644
--- a/hw/usb/hcd-ohci.h
+++ b/hw/usb/hcd-ohci.h
@@ -22,6 +22,7 @@
#define HCD_OHCI_H
#include "sysemu/dma.h"
+#include "hw/usb.h"
/* Number of Downstream Ports on the root hub: */
#define OHCI_MAX_PORTS 15
@@ -90,6 +91,21 @@ typedef struct OHCIState {
void (*ohci_die)(struct OHCIState *ohci);
} OHCIState;
+#define TYPE_SYSBUS_OHCI "sysbus-ohci"
+#define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
+
+typedef struct {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+
+OHCIState ohci;
+char *masterbus;
+uint32_t num_ports;
+uint32_t firstport;
+dma_addr_t dma_offset;
+} OHCISysBusState;
+
extern const VMStateDescription vmstate_ohci_state;
void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
--
2.17.1
[PATCH 2/3] arm: allwinner: Wire up USB OHCI
Initialize OHCI controllers on Allwinner A10. With this patch applied,
USB OHCI ports are discovered when booting the cubieboard machine with
a recent Linux kernel.
ohci-platform 1c14400.usb: Generic Platform OHCI controller
ohci-platform 1c14400.usb: new USB bus registered, assigned bus number 3
ohci-platform 1c14400.usb: irq 27, io mem 0x01c14400
ohci-platform 1c1c400.usb: Generic Platform OHCI controller
ohci-platform 1c1c400.usb: new USB bus registered, assigned bus number 4
ohci-platform 1c1c400.usb: irq 32, io mem 0x01c1c400
Signed-off-by: Guenter Roeck
---
hw/arm/allwinner-a10.c | 30 ++
include/hw/arm/allwinner-a10.h | 2 ++
2 files changed, 32 insertions(+)
diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 45cd8d2db5..818145428c 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -24,12 +24,15 @@
#include "hw/arm/allwinner-a10.h"
#include "hw/misc/unimp.h"
#include "sysemu/sysemu.h"
+#include "hw/boards.h"
+#include "hw/usb/hcd-ohci.h"
#define AW_A10_CCM_REG_BASE 0x01c2
#define AW_A10_PIC_REG_BASE 0x01c20400
#define AW_A10_PIT_REG_BASE 0x01c20c00
#define AW_A10_UART0_REG_BASE 0x01c28000
#define AW_A10_EMAC_BASE0x01c0b000
+#define AW_A10_OHCI_BASE0x01c14400
#define AW_A10_SATA_BASE0x01c18000
static void aw_a10_init(Object *obj)
@@ -53,6 +56,15 @@ static void aw_a10_init(Object *obj)
sysbus_init_child_obj(obj, "sata", >sata, sizeof(s->sata),
TYPE_ALLWINNER_AHCI);
+
+if (machine_usb(current_machine)) {
+int i;
+
+for (i = 0; i < ARRAY_SIZE(s->ohci); i++) {
+sysbus_init_child_obj(obj, "ohci[*]", OBJECT(>ohci[i]),
+ sizeof(s->ohci[i]), TYPE_SYSBUS_OHCI);
+}
+}
}
static void aw_a10_realize(DeviceState *dev, Error **errp)
@@ -133,6 +145,24 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
serial_mm_init(get_system_memory(), AW_A10_UART0_REG_BASE, 2,
qdev_get_gpio_in(dev, 1),
115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
+
+if (machine_usb(current_machine)) {
+int i;
+
+for (i = 0; i < ARRAY_SIZE(s->ohci); i++) {
+
+object_property_set_bool(OBJECT(>ohci[i]), true, "realized",
+ );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>ohci[i]), 0,
+AW_A10_OHCI_BASE + i * 0x8000);
+sysbus_connect_irq(SYS_BUS_DEVICE(>ohci[i]), 0,
+ qdev_get_gpio_in(dev, 64 + i));
+}
+}
}
static void aw_a10_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
index 0007a927bb..4864adbec3 100644
--- a/include/hw/arm/allwinner-a10.h
+++ b/include/hw/arm/allwinner-a10.h
@@ -9,6 +9,7 @@
#include "hw/intc/allwinner-a10-pic.h"
#include "hw/net/allwinner_emac.h"
#include "hw/ide/ahci.h"
+#include "hw/usb/hcd-ohci.h"
#include "target/arm/cpu.h"
@@ -29,6 +30,7 @@ typedef struct AwA10State {
AwEmacState emac;
AllwinnerAHCIState sata;
MemoryRegion sram_a;
+OHCISysBusState ohci[2];
} AwA10State;
#endif
--
2.17.1
[PATCH 2/2] hw/usb/hcd-ehci-sysbus: Remove obsolete xlnx, ps7-usb class
Xilinx USB devices are now instantiated through TYPE_CHIPIDEA,
and xlnx support in the EHCI code is no longer needed.
Signed-off-by: Guenter Roeck
---
hw/usb/hcd-ehci-sysbus.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 62612c9f5b..b5a014f968 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -114,22 +114,6 @@ static const TypeInfo ehci_platform_type_info = {
.class_init= ehci_platform_class_init,
};
-static void ehci_xlnx_class_init(ObjectClass *oc, void *data)
-{
-SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
-DeviceClass *dc = DEVICE_CLASS(oc);
-
-set_bit(DEVICE_CATEGORY_USB, dc->categories);
-sec->capsbase = 0x100;
-sec->opregbase = 0x140;
-}
-
-static const TypeInfo ehci_xlnx_type_info = {
-.name = "xlnx,ps7-usb",
-.parent= TYPE_SYS_BUS_EHCI,
-.class_init= ehci_xlnx_class_init,
-};
-
static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
{
SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -266,7 +250,6 @@ static void ehci_sysbus_register_types(void)
{
type_register_static(_type_info);
type_register_static(_platform_type_info);
-type_register_static(_xlnx_type_info);
type_register_static(_exynos4210_type_info);
type_register_static(_tegra2_type_info);
type_register_static(_ppc4xx_type_info);
--
2.17.1
[PATCH 1/2] hw/arm/xilinx_zynq: Fix USB port instantiation
USB ports must be instantiated as TYPE_CHIPIDEA to work.
Linux expects and checks various chipidea registers, which
do not exist with the basic ehci emulation.
Without this patch, USB ports fail to instantiate under Linux.
ci_hdrc ci_hdrc.0: doesn't support host
ci_hdrc ci_hdrc.0: no supported roles
With this patch, USB ports are instantiated, and it is possible
to boot from USB drive.
ci_hdrc ci_hdrc.0: EHCI Host Controller
ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1
ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
usb 1-1: new full-speed USB device number 2 using ci_hdrc
usb 1-1: not running at top speed; connect to a high speed hub
usb 1-1: config 1 interface 0 altsetting 0 endpoint 0x81 has invalid maxpacket
512, setting to 64
usb 1-1: config 1 interface 0 altsetting 0 endpoint 0x2 has invalid maxpacket
512, setting to 64
usb-storage 1-1:1.0: USB Mass Storage device detected
scsi host0: usb-storage 1-1:1.0
Signed-off-by: Guenter Roeck
---
hw/arm/xilinx_zynq.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3a0fa5b23f..b4a8b2f2c6 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -29,6 +29,7 @@
#include "hw/loader.h"
#include "hw/misc/zynq-xadc.h"
#include "hw/ssi/ssi.h"
+#include "hw/usb/chipidea.h"
#include "qemu/error-report.h"
#include "hw/sd/sdhci.h"
#include "hw/char/cadence_uart.h"
@@ -228,8 +229,8 @@ static void zynq_init(MachineState *machine)
zynq_init_spi_flashes(0xE0007000, pic[81-IRQ_OFFSET], false);
zynq_init_spi_flashes(0xE000D000, pic[51-IRQ_OFFSET], true);
-sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
-sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
+sysbus_create_simple(TYPE_CHIPIDEA, 0xE0002000, pic[53 - IRQ_OFFSET]);
+sysbus_create_simple(TYPE_CHIPIDEA, 0xE0003000, pic[76 - IRQ_OFFSET]);
cadence_uart_create(0xE000, pic[59 - IRQ_OFFSET], serial_hd(0));
cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hd(1));
--
2.17.1
[PATCH v3 1/2] target/arm: Support SError injection
This supports SError injection, which will be used by "virt" board to
simulating the behavior of NMI injection in next patch. As Peter Maydell
suggested, this adds a new interrupt (ARM_CPU_SERROR), which is parallel
to CPU_INTERRUPT_HARD. The backend depends on if kvm is enabled or not.
kvm_vcpu_ioctl(cpu, KVM_SET_VCPU_EVENTS) is leveraged to inject SError
or data abort to guest. When TCG is enabled, the behavior is simulated
by injecting SError and data abort to guest.
Signed-off-by: Gavin Shan
---
target/arm/cpu.c | 69 +++
target/arm/cpu.h | 17 ++-
target/arm/helper.c | 6
target/arm/m_helper.c | 8 +
4 files changed, 81 insertions(+), 19 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index b0762a76c4..180e29fb83 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -78,7 +78,7 @@ static bool arm_cpu_has_work(CPUState *cs)
&& cs->interrupt_request &
(CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
| CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ
- | CPU_INTERRUPT_EXITTB);
+ | ARM_CPU_SERROR | CPU_INTERRUPT_EXITTB);
}
void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
@@ -449,6 +449,9 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned
int excp_idx,
return false;
}
return !(env->daif & PSTATE_I);
+case EXCP_SERROR:
+ pstate_unmasked = !(env->daif & PSTATE_A);
+ break;
default:
g_assert_not_reached();
}
@@ -570,6 +573,16 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
goto found;
}
}
+
+if (interrupt_request & CPU_INTERRUPT_SERROR) {
+excp_idx = EXCP_SERROR;
+target_el = arm_phys_excp_target_el(cs, excp_idx, cur_el, secure);
+if (arm_excp_unmasked(cs, excp_idx, target_el,
+ cur_el, secure, hcr_el2)) {
+goto found;
+}
+}
+
return false;
found:
@@ -585,7 +598,7 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
CPUClass *cc = CPU_GET_CLASS(cs);
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = >env;
-bool ret = false;
+uint32_t excp_idx;
/* ARMv7-M interrupt masking works differently than -A or -R.
* There is no FIQ/IRQ distinction. Instead of I and F bits
@@ -594,13 +607,26 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
* (which depends on state like BASEPRI, FAULTMASK and the
* currently active exception).
*/
-if (interrupt_request & CPU_INTERRUPT_HARD
-&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
-cs->exception_index = EXCP_IRQ;
-cc->do_interrupt(cs);
-ret = true;
+if (!armv7m_nvic_can_take_pending_exception(env->nvic)) {
+return false;
+}
+
+if (interrupt_request & CPU_INTERRUPT_HARD) {
+excp_idx = EXCP_IRQ;
+goto found;
}
-return ret;
+
+if (interrupt_request & CPU_INTERRUPT_SERROR) {
+excp_idx = EXCP_SERROR;
+goto found;
+}
+
+return false;
+
+found:
+cs->exception_index = excp_idx;
+cc->do_interrupt(cs);
+return true;
}
#endif
@@ -656,7 +682,8 @@ static void arm_cpu_set_irq(void *opaque, int irq, int
level)
[ARM_CPU_IRQ] = CPU_INTERRUPT_HARD,
[ARM_CPU_FIQ] = CPU_INTERRUPT_FIQ,
[ARM_CPU_VIRQ] = CPU_INTERRUPT_VIRQ,
-[ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ
+[ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ,
+[ARM_CPU_SERROR] = CPU_INTERRUPT_SERROR,
};
if (level) {
@@ -676,6 +703,7 @@ static void arm_cpu_set_irq(void *opaque, int irq, int
level)
break;
case ARM_CPU_IRQ:
case ARM_CPU_FIQ:
+case ARM_CPU_SERROR:
if (level) {
cpu_interrupt(cs, mask[irq]);
} else {
@@ -693,8 +721,10 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int
level)
ARMCPU *cpu = opaque;
CPUARMState *env = >env;
CPUState *cs = CPU(cpu);
+struct kvm_vcpu_events events;
uint32_t linestate_bit;
int irq_id;
+bool inject_irq = true;
switch (irq) {
case ARM_CPU_IRQ:
@@ -705,6 +735,14 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int
level)
irq_id = KVM_ARM_IRQ_CPU_FIQ;
linestate_bit = CPU_INTERRUPT_FIQ;
break;
+case ARM_CPU_SERROR:
+if (!kvm_has_vcpu_events()) {
+return;
+}
+
+inject_irq = false;
+linestate_bit = CPU_INTERRUPT_SERROR;
+break;
default:
g_assert_not_reached();
}
@@ -714,7 +752,14 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int
level)
} else {
env->irq_line_state &= ~linestate_bit;
}
-kvm_arm_set_irq(cs->cpu_index, KVM_ARM_IRQ_TYPE_CPU, irq_id, !!level);
+
+if (inject_irq) {
+kvm_arm_set_irq(cs->cpu_index,
[PATCH v3 2/2] hw/arm/virt: Simulate NMI injection
This implements the backend to support HMP/QMP "nmi" command, which is
used to inject NMI interrupt to crash guest for debugging purpose. As
ARM architecture doesn't have NMI supported, so we're simulating the
behaviour by injecting SError or data abort to guest for "virt" board.
An additonal IRQ line is introduced for SError on each CPU. The IRQ line
is connected to SError exception handler. The IRQ line on CPU#0 is raised
when HMP/QMP "nmi" is issued to inject SError or data abort to crash guest.
Note the IRQ line can be shared with other devices who want to have the
capability of reporting errors in future.
Signed-off-by: Gavin Shan
---
hw/arm/virt.c | 34 +-
hw/intc/arm_gic_common.c | 3 +++
hw/intc/arm_gicv3_common.c | 3 +++
include/hw/intc/arm_gic_common.h | 1 +
include/hw/intc/arm_gicv3_common.h | 1 +
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f788fe27d6..78549faa75 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -71,6 +71,8 @@
#include "hw/mem/pc-dimm.h"
#include "hw/mem/nvdimm.h"
#include "hw/acpi/generic_event_device.h"
+#include "sysemu/hw_accel.h"
+#include "hw/nmi.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -690,7 +692,7 @@ static void create_gic(VirtMachineState *vms)
} else if (vms->virt) {
qemu_irq irq = qdev_get_gpio_in(vms->gic,
ppibase + ARCH_GIC_MAINT_IRQ);
-sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus, irq);
+sysbus_connect_irq(gicbusdev, i + 5 * smp_cpus, irq);
}
qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
@@ -704,6 +706,8 @@ static void create_gic(VirtMachineState *vms)
qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus,
+ qdev_get_gpio_in(cpudev, ARM_CPU_SERROR));
}
fdt_add_gic_node(vms);
@@ -2026,10 +2030,36 @@ static int virt_kvm_type(MachineState *ms, const char
*type_str)
return requested_pa_size > 40 ? requested_pa_size : 0;
}
+
+static void do_inject_serror(CPUState *cpu, run_on_cpu_data data)
+{
+VirtMachineState *vms = data.host_ptr;
+GICv3State *gicv3;
+GICState *gicv2;
+
+cpu_synchronize_state(cpu);
+
+if (vms->gic_version == 3) {
+gicv3 = ARM_GICV3_COMMON(OBJECT(vms->gic));
+qemu_irq_raise(gicv3->cpu[0].parent_serror);
+} else {
+gicv2 = ARM_GIC_COMMON(OBJECT(vms->gic));
+qemu_irq_raise(gicv2->parent_serror[0]);
+}
+}
+
+static void virt_inject_serror(NMIState *n, int cpu_index, Error **errp)
+{
+VirtMachineState *vms = VIRT_MACHINE(n);
+
+async_run_on_cpu(first_cpu, do_inject_serror, RUN_ON_CPU_HOST_PTR(vms));
+}
+
static void virt_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+NMIClass *nc = NMI_CLASS(oc);
mc->init = machvirt_init;
/* Start with max_cpus set to 512, which is the maximum supported by KVM.
@@ -2058,6 +2088,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
hc->unplug_request = virt_machine_device_unplug_request_cb;
mc->numa_mem_supported = true;
mc->auto_enable_numa_with_memhp = true;
+nc->nmi_monitor_handler = virt_inject_serror;
}
static void virt_instance_init(Object *obj)
@@ -2141,6 +2172,7 @@ static const TypeInfo virt_machine_info = {
.instance_init = virt_instance_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_HOTPLUG_HANDLER },
+ { TYPE_NMI },
{ }
},
};
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index e6c4fe7a5a..f39cefdeea 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -155,6 +155,9 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler
handler,
for (i = 0; i < s->num_cpu; i++) {
sysbus_init_irq(sbd, >parent_vfiq[i]);
}
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, >parent_serror[i]);
+}
if (s->virt_extn) {
for (i = 0; i < s->num_cpu; i++) {
sysbus_init_irq(sbd, >maintenance_irq[i]);
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 58ef65f589..19a04449a0 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -288,6 +288,9 @@ void gicv3_init_irqs_and_mmio(GICv3State *s,
qemu_irq_handler handler,
for (i = 0; i < s->num_cpu; i++) {
sysbus_init_irq(sbd, >cpu[i].parent_vfiq);
}
+for (i = 0; i < s->num_cpu; i++) {
+sysbus_init_irq(sbd, >cpu[i].parent_serror);
+}
[PATCH v3 0/2] hw/arm/virt: Simulate NMI Injection
This series simulates the behavior of receiving NMI interrupt for "virt" board. First of all, a new interrupt (SError) is supported for each CPU. The backend is either sending error events through kvm module or emulating the bahavior when TCG is enabled. The outcome is SError or data abort is raised to crash guest. For GICv2 or GICv3, a new IRQ line is added for each CPU and it's connected to the (above) introduced SError interrupt. The IRQ line of CPU#0 is raised when HMP/QMP "nmi" is issued, to crash the guest. Testing === After the HMP/QMP "nmi" is issued in the following 4 environment, the guest is crashed as expected. Accel Mode CrashedParameter kvm aarch64 yes-machine virt -cpu host kvm aarch32(cortex-a15) yes-machine virt -cpu host,aarch64=off tcg aarch64 yes-machine virt -cpu max tcg aarch32(cortex-a15) yes-machine virt -cpu cortex-a15 Changelog = v3: * Support SError injection for aarch32 (Richard Henderson) * Export the SError injection through IRQ line (Peter Maydell) * Removed RFC tag as it seems in correct track (Gavin Shan) v2: * Redesigned to fully exploit SError interrupt Gavin Shan (2): target/arm: Support SError injection hw/arm/virt: Simulate NMI injection hw/arm/virt.c | 34 ++- hw/intc/arm_gic_common.c | 3 ++ hw/intc/arm_gicv3_common.c | 3 ++ include/hw/intc/arm_gic_common.h | 1 + include/hw/intc/arm_gicv3_common.h | 1 + target/arm/cpu.c | 69 -- target/arm/cpu.h | 17 +--- target/arm/helper.c| 6 +++ target/arm/m_helper.c | 8 9 files changed, 122 insertions(+), 20 deletions(-) -- 2.23.0
Re: [RFC v3 03/25] hw/iommu: introduce IOMMUContext
On Wed, Feb 12, 2020 at 07:15:13AM +, Liu, Yi L wrote: > Hi Peter, > > > From: Peter Xu > > Sent: Wednesday, February 12, 2020 12:59 AM > > To: Liu, Yi L > > Subject: Re: [RFC v3 03/25] hw/iommu: introduce IOMMUContext > > > > On Fri, Jan 31, 2020 at 11:42:13AM +, Liu, Yi L wrote: > > > > I'm not very clear on the relationship betwen an IOMMUContext and a > > > > DualStageIOMMUObject. Can there be many IOMMUContexts to a > > > > DualStageIOMMUOBject? The other way around? Or is it just > > > > zero-or-one DualStageIOMMUObjects to an IOMMUContext? > > > > > > It is possible. As the below patch shows, DualStageIOMMUObject is per vfio > > > container. IOMMUContext can be either per-device or shared across devices, > > > it depends on vendor specific vIOMMU emulators. > > > > Is there an example when an IOMMUContext can be not per-device? > > No, I don’t have such example so far. But as IOMMUContext is got from > pci_device_iommu_context(), in concept it possible to be not per-device. > It is kind of leave to vIOMMU to decide if different devices could share a > single IOMMUContext. On the "pseries" machine the vIOMMU only has one set of translations for a whole virtual PCI Host Bridge (vPHB). So if you attach multiple devices to a single vPHB, I believe you'd get multiple devices in an IOMMUContext. Well.. if we did the PASID stuff, which we don't at the moment. Note that on pseries on the other hand it's routine to create multiple vPHBs, rather than multiple PCI roots being an oddity as it is on x86. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: VW ELF loader
On Mon, Feb 10, 2020 at 12:26:07PM +0100, Paolo Bonzini wrote: > On 10/02/20 08:28, David Gibson wrote: > > On Thu, Feb 06, 2020 at 09:27:01AM +0100, Paolo Bonzini wrote: > >> On 05/02/20 07:06, David Gibson wrote: > >>> On Tue, Feb 04, 2020 at 12:26:32AM +0100, Paolo Bonzini wrote: > >> I'm really sorry if what I am saying is stupid; but I was thinking of a > >> firmware entrypoint like > >> > >>if (op == "read" || op == "write") > >>do_driver_stuff(op); > >>else > >>hypercall(); > > > > Um... I'm not really clear on where you're imagining this going. In > > the OF model, device operations are done by "opening" a device tree > > node then executing methods on it, so you can't really even get to > > this point without a bunch of DT stuff. > > Could you delegate that part to QEMU, as in the v6 patches? The > firmware would record the path<->ihandle association on open and close, > and then you can use that when GRUB does "read" and "write" to invoke > the appropriate driver. > > >> This is not even close to pseudocode, but hopefully enough to give the > >> idea. Perhaps what I don't understand is why you can't start the > >> firmware with r3 pointing to the device tree, and stash it for when you > >> leave control to GRUB. > > > > Again, I'm not even really sure what you mean by this. We already > > enter SLOF with r3 pointing to a device tree. I'm not sure what > > stashing it would accomplish. GRUB as it stands expects an OF style > > entry point though, not a flat tree style entry point. > > Again, sorry if what I'm saying makes little sense. The terminology is > certainly off. What I mean is: > > - read the device tree, instantiate all PCI and virtio drivers > > - keep the device tree around for use while GRUB is running > > - find and invoke GRUB > > - on the OF entry point, wrap open and close + handle the disk and > network entry points, and pass everything else to QEMU. > > >> The TTY can use the simple > >> getchar/putchar hypercalls, > > > > Yes... though if people attach a graphical console they might be > > pretty surprised that they don't get anything on there. > > They wouldn't with Alexey's code either, would they? And it would be > yet another QEMU backend to hook into, while with firmware it would be > lots of code to write but super-boring and something that has been done > countless times. That's a fair point. > > We can possibly ignore the spapr virtual devices. They seemed like > > they'd be important for people transitioning from guests under > > PowerVM, but honestly I'm not sure they've ever been used much. > > > > We do support emulated (or passthrough) PCI devices. I don't know if > > they're common enough that we need boot support for them. Netboot > > from a vfio network adaptor might be something people want. > > Can you get that with SLOF? I think yes, if your passthrough device is one of the small number supported by SLOF. > > USB storage is also a fairly likely candidate, and that would add a > > *lot* of extra complexity, since we'd need both the HCD and storage > > drivers. > > Any reason to make it USB and not a virtio-blk device? (On x86 these > days you only add USB storage disks to a VM in order to get drivers to > Windows). Hm, yeah, maybe. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[Bug 1863200] [NEW] Reconnect failed with loopback virtio1.1 server mode test
Public bug reported: Issue discription： Packed ring server mode is a new feature to enable the virtio-user or virtio-pmd(in VM) as the server, vhost as the client, then when the vhost-user is killed then re-launched, the vhost-user can connect back to virtio-user/virtio-pmd again. Test with dpdk 20.02 ,virtio-pmd loopback reconnect from vhost-user failed. Test Environment: DPDK version: DPDK v20.02 Other software versions: virtio1.1 Qemu versions:4.2.0 OS: Linux 4.15.0-20-generic Compiler: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0 Hardware platform: R2208WFTZSR. The reproduce step is : Test Case: vhost-user/virtio-pmd loopback reconnect from vhost-user === Flow: Vhost-user --> Virtio --> Vhost-user 1. Launch vhost-user with client mode by below commands:: ./testpmd -c 0x30 -n 4 --socket-mem 1024,1024 --legacy-mem --vdev 'eth_vhost0,iface=/tmp/vhost-net,client=1,queues=1' -- -i --nb-cores=1 testpmd>set fwd mac 2. Start VM with 1 virtio device, and set the qemu as server mode:: ./qemu-system-x86_64 -name vm2 -enable-kvm -cpu host -smp 100 -m 8G \ -object memory-backend-file,id=mem,size=8192M,mem-path=/mnt/huge,share=on \ -numa node,memdev=mem -mem-prealloc -drive file=/home/xuan/dpdk_project/shell/u18.img \ -chardev socket,path=/tmp/vm2_qga0.sock,server,nowait,id=vm2_qga0 -device virtio-serial \ -device virtserialport,chardev=vm2_qga0,name=org.qemu.guest_agent.2 -daemonize \ -monitor unix:/tmp/vm2_monitor.sock,server,nowait -net nic,macaddr=00:00:00:08:e8:aa,addr=1f \ -net user,hostfwd=tcp:127.0.0.1:6002-:22 \ -chardev socket,id=char0,path=/tmp/vhost-net,server \ -netdev type=vhost-user,id=netdev0,chardev=char0,vhostforce \ -device virtio-net-pci,netdev=netdev0,mac=52:54:00:00:00:01,mrg_rxbuf=on,rx_queue_size=1024,tx_queue_size=1024,packed=on \ -vnc :10 3. On VM, bind virtio net to igb_uio and run testpmd:: ./testpmd -c 0x3 -n 4 -- -i --nb-cores=1 --txd=1024 --rxd=1024 testpmd>set fwd mac testpmd>start 4. Send packets by vhost-user, check if packets can be RX/TX with virtio-pmd:: testpmd>start tx_first 32 testpmd>show port stats all 5. On host, quit vhost-user, then re-launch the vhost-user with below command:: testpmd>quit ./testpmd -c 0x30 -n 4 --socket-mem 1024,1024 --legacy-mem --vdev 'eth_vhost0,iface=/tmp/vhost-net,client=1,queues=1' -- -i --nb-cores=1 testpmd>set fwd mac testpmd>start tx_first 32 6. Check if the reconnection can work, still send packets by vhost-user, check if packets can be RX/TX with virtio-pmd:: testpmd>show port stats all Expected result:: After the vhost-user is killed then re-launched, the VM can connect back to vhost-user again with throughput. Real result:: After the vhost-user is killed then re-launched, no throughput with PVP. Analysis:: QEMU has its own way to handle reconnect function for virtio server mode. However, for packed ring, when reconnecting to virtio, vhost cannot get the status of descriptors via the descriptor ring. This bug is caused since the reconnection for packed ring need additional reset operation. ** Affects: qemu Importance: Undecided Status: New ** Tags: mode packed ring server -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1863200 Title: Reconnect failed with loopback virtio1.1 server mode test Status in QEMU: New Bug description: Issue discription： Packed ring server mode is a new feature to enable the virtio-user or virtio-pmd(in VM) as the server, vhost as the client, then when the vhost-user is killed then re-launched, the vhost-user can connect back to virtio-user/virtio-pmd again. Test with dpdk 20.02 ,virtio-pmd loopback reconnect from vhost-user failed. Test Environment: DPDK version: DPDK v20.02 Other software versions: virtio1.1 Qemu versions:4.2.0 OS: Linux 4.15.0-20-generic Compiler: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0 Hardware platform: R2208WFTZSR. The reproduce step is : Test Case: vhost-user/virtio-pmd loopback reconnect from vhost-user === Flow: Vhost-user --> Virtio --> Vhost-user 1. Launch vhost-user with client mode by below commands:: ./testpmd -c 0x30 -n 4 --socket-mem 1024,1024 --legacy-mem --vdev 'eth_vhost0,iface=/tmp/vhost-net,client=1,queues=1' -- -i --nb-cores=1 testpmd>set fwd mac 2. Start VM with 1 virtio device, and set the qemu as server mode:: ./qemu-system-x86_64 -name vm2 -enable-kvm -cpu host -smp 100 -m 8G \ -object memory-backend-file,id=mem,size=8192M,mem-path=/mnt/huge,share=on \ -numa node,memdev=mem -mem-prealloc -drive file=/home/xuan/dpdk_project/shell/u18.img \ -chardev socket,path=/tmp/vm2_qga0.sock,server,nowait,id=vm2_qga0 -device virtio-serial \
[PATCH] migration/throttle: Make throttle slower at tail stage
At the tail stage of throttle, VM is very sensitive to
CPU percentage. We just throttle 30% of remaining CPU
when throttle is more than 80 percentage.
This doesn't conflict with cpu_throttle_increment.
This may make migration time longer, and is disabled
by default.
Signed-off-by: Keqian Zhu
---
Cc: Juan Quintela
Cc: "Dr. David Alan Gilbert"
Cc: Eric Blake
Cc: Markus Armbruster
---
migration/migration.c | 13 +
migration/ram.c | 18 --
monitor/hmp-cmds.c| 4
qapi/migration.json | 21 +
4 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 3a21a4686c..37b569cee9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -782,6 +782,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
**errp)
params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
params->has_cpu_throttle_increment = true;
params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
+params->has_cpu_throttle_tailslow = true;
+params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
params->has_tls_creds = true;
params->tls_creds = g_strdup(s->parameters.tls_creds);
params->has_tls_hostname = true;
@@ -1287,6 +1289,10 @@ static void
migrate_params_test_apply(MigrateSetParameters *params,
dest->cpu_throttle_increment = params->cpu_throttle_increment;
}
+if (params->has_cpu_throttle_tailslow) {
+dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+}
+
if (params->has_tls_creds) {
assert(params->tls_creds->type == QTYPE_QSTRING);
dest->tls_creds = g_strdup(params->tls_creds->u.s);
@@ -1368,6 +1374,10 @@ static void migrate_params_apply(MigrateSetParameters
*params, Error **errp)
s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
}
+if (params->has_cpu_throttle_tailslow) {
+s->parameters.cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+}
+
if (params->has_tls_creds) {
g_free(s->parameters.tls_creds);
assert(params->tls_creds->type == QTYPE_QSTRING);
@@ -3504,6 +3514,8 @@ static Property migration_properties[] = {
DEFINE_PROP_UINT8("x-cpu-throttle-increment", MigrationState,
parameters.cpu_throttle_increment,
DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT),
+DEFINE_PROP_BOOL("x-cpu-throttle-tailslow", MigrationState,
+ parameters.cpu_throttle_tailslow, false),
DEFINE_PROP_SIZE("x-max-bandwidth", MigrationState,
parameters.max_bandwidth, MAX_THROTTLE),
DEFINE_PROP_UINT64("x-downtime-limit", MigrationState,
@@ -3600,6 +3612,7 @@ static void migration_instance_init(Object *obj)
params->has_decompress_threads = true;
params->has_cpu_throttle_initial = true;
params->has_cpu_throttle_increment = true;
+params->has_cpu_throttle_tailslow = true;
params->has_max_bandwidth = true;
params->has_downtime_limit = true;
params->has_x_checkpoint_delay = true;
diff --git a/migration/ram.c b/migration/ram.c
index ed23ed1c7c..d86413a5d1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -29,6 +29,7 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include
+#include
#include "qemu/cutils.h"
#include "qemu/bitops.h"
#include "qemu/bitmap.h"
@@ -620,15 +621,28 @@ static void mig_throttle_guest_down(void)
{
MigrationState *s = migrate_get_current();
uint64_t pct_initial = s->parameters.cpu_throttle_initial;
-uint64_t pct_icrement = s->parameters.cpu_throttle_increment;
+uint64_t pct_increment = s->parameters.cpu_throttle_increment;
+bool pct_tailslow = s->parameters.cpu_throttle_tailslow;
int pct_max = s->parameters.max_cpu_throttle;
+const uint64_t cpu_throttle_now = cpu_throttle_get_percentage();
+uint64_t cpu_throttle_inc;
+
/* We have not started throttling yet. Let's start it. */
if (!cpu_throttle_active()) {
cpu_throttle_set(pct_initial);
} else {
/* Throttling already on, just increase the rate */
-cpu_throttle_set(MIN(cpu_throttle_get_percentage() + pct_icrement,
+if (cpu_throttle_now >= 80 && pct_tailslow) {
+/* Eat some scale of CPU from remaining */
+cpu_throttle_inc = ceil((100 - cpu_throttle_now) * 0.3);
+if (cpu_throttle_inc > pct_increment) {
+cpu_throttle_inc = pct_increment;
+}
+} else {
+cpu_throttle_inc = pct_increment;
+}
+cpu_throttle_set(MIN(cpu_throttle_now + cpu_throttle_inc,
pct_max));
}
}
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 558fe06b8f..b5f5c0b382 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -416,6 +416,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict
*qdict)
Re: VW ELF loader
On Fri, Feb 14, 2020 at 11:01:26AM +1100, Alexey Kardashevskiy wrote: > > > On 13/02/2020 21:17, Paolo Bonzini wrote: > > On 13/02/20 02:43, Alexey Kardashevskiy wrote: > >> > >> Ok. So, I have made a small firmware which does OF CI, loads GRUB and > >> instantiates RTAS: > >> https://github.com/aik/of1275 > >> Quite raw but gives the idea. > >> > >> It does not contain drivers and still relies on QEMU to hook an OF path > >> to a backend. Is this a showstopper and without drivers it is no go? > >> Thanks, > > > > Yes, it's really the drivers. Something like netboot wouldn't work for > > example. > > > > I don't have a problem with relying on QEMU for opening and closing OF > > paths, but I really believe that read/write on ihandles should be done > > within the firmware and not QEMU. > > Moving read/write to the firmware is not a problem but there is a little > mix up here :) > > An ihandle is open from a path and nothing there suggests drivers, it is > up to the ihandle's "read" method what happens next. > > If we do PCI drivers in the firmware, then the entire ihandle (== > "opened instance of a phandle") business goes to the firmware and we are > slowly bringing the existing mess back again. Right, even setting aside the specifics of how ihandles are managed, having the device tree in qemu but device handling in firmware seems like an even more complex interaction between qemu and firmware pieces of the environment, which is what we've been trying to avoid here. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: VW ELF loader
On Mon, Feb 10, 2020 at 12:25:39PM +0100, Paolo Bonzini wrote: > On 10/02/20 08:30, David Gibson wrote: > >> Anything you put in the host is potential attack surface. > > Ok, it is attack surface you're concerned about. That wasn't totally > > clear before this point. > > Part that, part having to add backend hooks that weren't needed so far. > > >> Plus, you're not doing a different thing than anyone else and as > >> you've found out it may be easy for block device but not for > >> everything else. > > > > Uh.. was that supposed to be "we *are* doing a different thing than > > anyone else"? > > Alexey's question was "what is exactly the benefit", so "not doing a > different thing" is the answer (one of them). Ah, right, I see. > >> Every platform that QEMU supports is just using a firmware to do > >> firmware things; it can be U-Boot, EDK-2, SLOF, SeaBIOS, qboot, with > >> varying level of complexity. Some are doing -kernel in QEMU rather than > >> firmware, but that's where things end. > > > > Well, yeah, but AIUI those platforms actually have a defined hardware > > environment on which the firmware is running. For PAPR we don't, we > > *only* have a specification for the "hardware"+"firmware" environment > > as seen by the OS together. > > PAPR is a specification for the environment as seen by the OS. But "-M > pseries" is already a defined hardware environment on which SLOF is > running. "defined" might be a bit strong. We have on multiple occasions required synchronized SLOF and qemu updates in order to keep presenting the same guest environment. > There's nothing that prevents you from defining more of that > environment in order to run Linux (for petitboot) or your own > pseudo-OpenFirmware driver provider inside it. Well, sure, but we don't want that definition to introduce lots of complexity we have to maintain on top of the existing HV and firmware interfaces that are defined. I realized what I said about the firmware interfaces requiring HV privilege was a bit misleading. For the boot time firmware components, such as the OF client interface, that's mostly not true (with the big and hairy exception of the ibm,client-architecture-support feature negotiation mechanism). It is, however, true of the runtime RTAS interfaces. In fact it's true to the point that, at least for most of the RTAS interfaces we care about, it's hard to imagine an in-guest implementation doing anything much other than repackaging the information it gets and forwarding it to the hypervisor. For most purposes RTAS is pretty much an alternative hypercall mechanism. So, I think implementing the RTAS entirely in qemu was the right choice. Where it gets complicated is that a number of RTAS calls need to match IDs with stuff from the boot time firmware. Particularly phandles of device nodes, and some other IDs. Which would be fine if the boot time firmware took the device tree from qemu and used it unmodified. But.. it doesn't, quite. In particular it assigns its own phandles, because it uses them as an internal index. And worse, clients can alter the device tree, and this is used to a small extent - grub pokes a few values in there for use later. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[PATCH] ppc: free 'fdt' after reset the machine
From: Pan Nengyuan 'fdt' forgot to clean both e500 and pnv when we call 'system_reset' on ppc, this patch fix it. The leak stacks are as follow: Direct leak of 4194304 byte(s) in 4 object(s) allocated from: #0 0x7fafe37dd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970) #1 0x7fafe2e3149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d) #2 0x561876f7f80d in create_device_tree /mnt/sdb/qemu-new/qemu/device_tree.c:40 #3 0x561876b7ac29 in ppce500_load_device_tree /mnt/sdb/qemu-new/qemu/hw/ppc/e500.c:364 #4 0x561876b7f437 in ppce500_reset_device_tree /mnt/sdb/qemu-new/qemu/hw/ppc/e500.c:617 #5 0x56187718b1ae in qemu_devices_reset /mnt/sdb/qemu-new/qemu/hw/core/reset.c:69 #6 0x561876f6938d in qemu_system_reset /mnt/sdb/qemu-new/qemu/vl.c:1412 #7 0x561876f6a25b in main_loop_should_exit /mnt/sdb/qemu-new/qemu/vl.c:1645 #8 0x561876f6a398 in main_loop /mnt/sdb/qemu-new/qemu/vl.c:1679 #9 0x561876f7da8e in main /mnt/sdb/qemu-new/qemu/vl.c:4438 #10 0x7fafde16b812 in __libc_start_main ../csu/libc-start.c:308 #11 0x5618765c055d in _start (/mnt/sdb/qemu-new/qemu/build/ppc64-softmmu/qemu-system-ppc64+0x2b1555d) Direct leak of 1048576 byte(s) in 1 object(s) allocated from: #0 0x7fc0a6f1b970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970) #1 0x7fc0a656f49d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d) #2 0x55eb05acd2ca in pnv_dt_create /mnt/sdb/qemu-new/qemu/hw/ppc/pnv.c:507 #3 0x55eb05ace5bf in pnv_reset /mnt/sdb/qemu-new/qemu/hw/ppc/pnv.c:578 #4 0x55eb05f2f395 in qemu_system_reset /mnt/sdb/qemu-new/qemu/vl.c:1410 #5 0x55eb05f43850 in main /mnt/sdb/qemu-new/qemu/vl.c:4403 #6 0x7fc0a18a9812 in __libc_start_main ../csu/libc-start.c:308 #7 0x55eb0558655d in _start (/mnt/sdb/qemu-new/qemu/build/ppc64-softmmu/qemu-system-ppc64+0x2b1555d) Reported-by: Euler Robot Signed-off-by: Pan Nengyuan --- hw/ppc/e500.c | 1 + hw/ppc/pnv.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 886442e54f..af537bba2b 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -594,6 +594,7 @@ done: cpu_physical_memory_write(addr, fdt, fdt_size); } ret = fdt_size; +g_free(fdt); out: g_free(pci_map); diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 139c857b1e..e98038b809 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -582,6 +582,8 @@ static void pnv_reset(MachineState *machine) qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt)); + +g_free(fdt); } static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp) -- 2.18.2
Re: [PATCH v2 1/5] vhost-user block device backend
Thank you for reviewing this patch! I'm already posted v3 based on
your feedback.
> > +#include "hw/qdev-properties.h"
> > +enum {
> > +VHOST_USER_BLK_MAX_QUEUES = 8,
> > +};
> The number of queues is hardcoded to 1 so this constant can be removed
for now.
> > +
> > +static QTAILQ_HEAD(, VubDev) vub_devs = QTAILQ_HEAD_INITIALIZER(vub_devs);
> I'm not sure if this list will be necessary. See my comments about
> replacing the "name" property with Object's built-in "id" property.
There is no need to start vhost-user-blk-server with the same exported
drive since vhost-user-blk-server can serve multiple clients
simutanously. And we shoudn't started two vhost-user-blk-servers with
the same unix socket path. So we need this list to avoid dupliate
servers. And as pointed out by Kevin, "name" property actually means
node-name which is used in v3.
> > +#include "hw/qdev-properties.h"
> > +enum {
> > +VHOST_USER_BLK_MAX_QUEUES = 8,
> > +};
> The number of queues is hardcoded to 1 so this constant can be removed
for now.
I've set VHOST_USER_BLK_MAX_QUEUES = 1 in v3 to avoid magic number.
> > +config->seg_max = 128 - 2;
> This is okay for now but should be changed in the future.
> hw/block/virtio-blk.c was recently modified to calculate seg_max based
> on the virtqueue size (which is where the hardcoded 128 originally came
> from). Applications that use large buffer sizes (128+ KB) benefit from
> larger virtqueue sizes and seg_max.
I've looked at the implementation of "-device
vhost-user-blk-pci,queue-size=512" regarding seg_max and found out
vhost-user-blk-server doesn' have the chance to caculate seg_max based
on the virtqueue size and report it back to QEMU as vhost-user client
in time.
QEMU as vhost-user client will create virtqueues with the size of
queue-size parameter. Later it will get the configureation including
seg_max from vhost-user-blk-server by sending VHOST_USER_SET_CONFIG
message and this seg_max vallue will be sent to guest OS. Guest OS
will set the real size of virtqueue which will be sent to
vhost-user-blk-server through VHOST_USER_SET_VRING_NUM message. After
that QEMU as vhost-user client will never send VHOST_USER_SET_CONFIG
again.
There could be three ways to address this issue,
1. Add seg_max_adjust and queue-size object property for
vhost-user-blk device in a way similar to virtio-blk device. And QEMU
as vhost-user client will ignore seg_max parameter from
vhost-user-blk-server. It will caculate seg_max based on queue-size
and report it to guest OS.
2. Add seg_max_adjust and queue-size object property for
vhost-user-blk server and let vhost-user-blk server calculate seg_max
based on queue-size
3. Let QEMU as vhost-user client tell vhost-user-blk-server its queue
size by sending VHOST_USER_SET_VRING_NUM message first. When
vhost-user-blk-server receive the VHOST_USER_SET_CONFIG message, it
will calculate seg_max and report it back to QEMU as vhost-user
client.
Which way do you is the best?
> > +void vub_accept(QIONetListener *listener, QIOChannelSocket *sioc,
> > +gpointer opaque)
> > +{
> > +VuClient *client;
> > +VubDev *vub_device = opaque;
> > +client = g_new0(VuClient, 1);
> Is it helpful to have a separate VuClient struct even though only a
> single vhost-user client can be connected at a time? It may be simpler
> to keep connection state directly in VubDev.
Currently, I don't use chardev as an object property of
vhost-user-blk-server. So actually multiple clients can be connected
simutaneously.
All the other suggestions have been adopted in v3. Thank you for your advice!
On Thu, Jan 16, 2020 at 9:51 PM Stefan Hajnoczi wrote:
>
> On Tue, Jan 14, 2020 at 10:06:16PM +0800, Coiby Xu wrote:
> > By making use of libvhost, multiple block device drives can be exported and
> > each drive can serve multiple clients simultaneously. Since
> > vhost-user-server needs a block drive to be created first, delay the
> > creation of this object.
> >
> > Signed-off-by: Coiby Xu
> > ---
> > blockdev-vu.c | 1008
>
> This file contains both vhost-user-blk code and generic vhost-user
> protocol code. Please split them into two files:
> 1. backends/vhost-user-blk-server.c
> 2. util/vhost-user-server.c
>
> (As I read and understand the code better I may have better suggestions
> about where to put these source files and how to name them.)
>
> > include/block/vhost-user.h | 46 ++
> > vl.c |4 +
> > 3 files changed, 1058 insertions(+)
> > create mode 100644 blockdev-vu.c
> > create mode 100644 include/block/vhost-user.h
> >
> > diff --git a/blockdev-vu.c b/blockdev-vu.c
> > new file mode 100644
> > index 00..45f0bb43a7
> > --- /dev/null
> > +++ b/blockdev-vu.c
> > @@ -0,0 +1,1008 @@
> > +#include "qemu/osdep.h"
> > +#include "block/vhost-user.h"
> > +#include "qapi/error.h"
> > +#include "qapi/qapi-types-sockets.h"
> > +#include
Re: [PATCH 3/3] spapr: Migrate SpaprDrc::unplug_requested
On Mon, Feb 03, 2020 at 11:36:22PM +0100, Greg Kurz wrote:
> Hot unplugging a device is an asynchronous operation. If the guest is
> migrated after the event was sent but before it could release the
> device with RTAS, the destination QEMU doesn't know about the pending
> unplug operation and doesn't actually remove the device when the guest
> finally releases it. The device
>
> Migrate SpaprDrc::unplug_requested to fix the inconsistency. This is
> done with a subsection that is only sent if an unplug request is
> pending. This allows to preserve migration with older guests in the
> case of a pending hotplug request. This will cause migration to fail
> if the destination can't handle the subsection, but this is better
> than ending with an inconsistency.
>
> Signed-off-by: Greg Kurz
> ---
> hw/ppc/spapr_drc.c | 27 +--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index d512ac6e1e7f..6f5cab70fc6b 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -455,6 +455,22 @@ void spapr_drc_reset(SpaprDrc *drc)
> }
> }
>
> +static bool spapr_drc_unplug_requested_needed(void *opaque)
> +{
> +return spapr_drc_unplug_requested(opaque);
> +}
> +
> +static const VMStateDescription vmstate_spapr_drc_unplug_requested = {
> +.name = "spapr_drc/unplug_requested",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = spapr_drc_unplug_requested_needed,
> +.fields = (VMStateField []) {
> +VMSTATE_BOOL(unplug_requested, SpaprDrc),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> static bool spapr_drc_needed(void *opaque)
> {
> SpaprDrc *drc = (SpaprDrc *)opaque;
> @@ -467,8 +483,11 @@ static bool spapr_drc_needed(void *opaque)
> /*
> * We need to migrate the state if it's not equal to the expected
> * long-term state, which is the same as the coldplugged initial
> - * state */
> -return !spapr_drc_device_ready(drc);
> + * state, or if an unplug request is pending.
> + */
> +return
> +spapr_drc_unplug_requested_needed(drc) ||
> +!spapr_drc_device_ready(drc);
Hrm. You start the series by splitting spapr_drc_device_ready() from
spapr_drc_needed(). But at this point, I'm pretty sure you've now got
all the callers of spapr_drc_device_ready() doing equivalent logic
about them, so they might as well be one function again. That seems
pretty roundabout.
I don't think the rationale for not using the drc_ready function from
the CAS path really makes sense. It's not just an accident that those
use the same logic - in both cases what we're testing is "Is the DRC
in a state other than that of a default cold-plugged device?".
Changing the name might be sensible, but I still think we want a
common function for the two cases.
> }
>
> static const VMStateDescription vmstate_spapr_drc = {
> @@ -479,6 +498,10 @@ static const VMStateDescription vmstate_spapr_drc = {
> .fields = (VMStateField []) {
> VMSTATE_UINT32(state, SpaprDrc),
> VMSTATE_END_OF_LIST()
> +},
> +.subsections = (const VMStateDescription * []) {
> +_spapr_drc_unplug_requested,
> +NULL
> }
> };
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 0/3] spapr: Fix device unplug vs CAS or migration
On Thu, Feb 13, 2020 at 04:10:55PM +0100, Greg Kurz wrote: > Ping ? > > This series fixes actual bugs. Also, I have another patch on top of > that to cold plug (or remove) devices pending hot plug (or unplug) > before CAS, hence removing the need for CAS reboot in these cases. > This requires SLOF to correctly parse the FDT it gets at CAS. Patches > have been sent for that too: > > https://git.qemu.org/?p=SLOF.git;a=commitdiff;h=689ff6f6554d94fdab854bf4fc4ec85e2675e43d > https://git.qemu.org/?p=SLOF.git;a=commitdiff;h=a093be1ebe7a48321646601d94be6cf735c81e12 > https://patchwork.ozlabs.org/patch/1235817/ Yeah, sorry, I've been having a bit of trouble getting my head around the cases here. I've sent a comment now. > > On Mon, 03 Feb 2020 23:36:04 +0100 > Greg Kurz wrote: > > > While working on getting rid of CAS reboot, I realized that we currently > > don't handle device hot unplug properly in the following situations: > > > > 1) if the device is unplugged between boot and CAS, SLOF doesn't handle > >the even, which is a known limitation. The device hence stays around > >forever (specifically, until some other event is emitted and the guest > >eventually completes the unplug or a reboot). Until we can teach SLOF > >to correctly process the full FDT at CAS, we should trigger a CAS reboot, > >like we already do for hotplug. > > > > 2) if the guest is migrated after the even was emitted but before the > >guest could process it, the destination is unaware of the pending > >unplug operation and doesn't remove the device when the guests > >releases it. The 'unplug_requested' field of the DRC is actually state > >that should be migrated. > > > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [RESEND RFC PATCH v2 1/2] target/arm: Allow to inject SError interrupt
On 2/13/20 9:31 PM, Peter Maydell wrote: On Thu, 13 Feb 2020 at 03:49, Gavin Shan wrote: On 2/12/20 10:34 PM, Peter Maydell wrote: Yeah, this is on my list to look at; Richard Henderson also could have a look at it. From a quick scan I suspect you may be missing handling for AArch32. Yes, the functionality is only supported on aarch64 currently by intention because the next patch enables it on "max" and "host" CPU models and both of them are running in aarch64 mode. https://patchwork.kernel.org/patch/11366119/ If you really want to get this supported for aarch32 either, I can do it. However, it seems there is a long list of aarch32 CPU models, defined in target/arm/cpu.c::arm_cpus. so which CPU models you prefer to see with this supported? I think we might choose one or two popular CPU models if you agree. I don't think you should need to care about the CPU models. We should implement SError (aka "asynchronous external abort" in ARMv7 and earlier) generically for all CPUs. The SError/async abort should be triggered by a qemu_irq line inbound to the CPU (similar to FIQ and IRQ); the board can choose to wire that up to something, or not, as it likes. Yes, what I need to care about is board with this. It means I will get this supported on "virt" only. Thanks for the comments and the changes will be included in v3. Thanks, Gavin
Re: [PATCH v2 14/19] target/riscv: progressively load the instruction during decode
On Thu, Feb 13, 2020 at 3:08 PM Alex Bennée wrote:
>
> The plugin system would throw up a harmless warning when it detected
> that a disassembly of an instruction didn't use all it's bytes. Fix
> the riscv decoder to only load the instruction bytes it needs as it
> needs them.
>
> This drops opcode from the ctx in favour if passing the appropriately
> sized opcode down a few levels of the decode.
>
> Signed-off-by: Alex Bennée
> Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
Alistair
>
> ---
> v2
> - use extract16 for uint16_t opcodes
>
> squash! target/riscv: progressively load the instruction during decode
> ---
> target/riscv/instmap.h | 8
> target/riscv/translate.c | 40 +---
> 2 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/target/riscv/instmap.h b/target/riscv/instmap.h
> index f8ad7d60fd5..40b6d2b64de 100644
> --- a/target/riscv/instmap.h
> +++ b/target/riscv/instmap.h
> @@ -344,8 +344,8 @@ enum {
> #define GET_C_LW_IMM(inst) ((extract32(inst, 6, 1) << 2) \
> | (extract32(inst, 10, 3) << 3) \
> | (extract32(inst, 5, 1) << 6))
> -#define GET_C_LD_IMM(inst) ((extract32(inst, 10, 3) << 3) \
> -| (extract32(inst, 5, 2) << 6))
> +#define GET_C_LD_IMM(inst) ((extract16(inst, 10, 3) << 3) \
> +| (extract16(inst, 5, 2) << 6))
> #define GET_C_J_IMM(inst) ((extract32(inst, 3, 3) << 1) \
> | (extract32(inst, 11, 1) << 4) \
> | (extract32(inst, 2, 1) << 5) \
> @@ -363,7 +363,7 @@ enum {
> #define GET_C_RD(inst) GET_RD(inst)
> #define GET_C_RS1(inst) GET_RD(inst)
> #define GET_C_RS2(inst) extract32(inst, 2, 5)
> -#define GET_C_RS1S(inst)(8 + extract32(inst, 7, 3))
> -#define GET_C_RS2S(inst)(8 + extract32(inst, 2, 3))
> +#define GET_C_RS1S(inst)(8 + extract16(inst, 7, 3))
> +#define GET_C_RS2S(inst)(8 + extract16(inst, 2, 3))
>
> #endif
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 14dc71156be..d5de7f468a7 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -44,7 +44,6 @@ typedef struct DisasContext {
> /* pc_succ_insn points to the instruction following base.pc_next */
> target_ulong pc_succ_insn;
> target_ulong priv_ver;
> -uint32_t opcode;
> uint32_t mstatus_fs;
> uint32_t misa;
> uint32_t mem_idx;
> @@ -492,45 +491,45 @@ static void gen_set_rm(DisasContext *ctx, int rm)
> tcg_temp_free_i32(t0);
> }
>
> -static void decode_RV32_64C0(DisasContext *ctx)
> +static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> {
> -uint8_t funct3 = extract32(ctx->opcode, 13, 3);
> -uint8_t rd_rs2 = GET_C_RS2S(ctx->opcode);
> -uint8_t rs1s = GET_C_RS1S(ctx->opcode);
> +uint8_t funct3 = extract16(opcode, 13, 3);
> +uint8_t rd_rs2 = GET_C_RS2S(opcode);
> +uint8_t rs1s = GET_C_RS1S(opcode);
>
> switch (funct3) {
> case 3:
> #if defined(TARGET_RISCV64)
> /* C.LD(RV64/128) -> ld rd', offset[7:3](rs1')*/
> gen_load_c(ctx, OPC_RISC_LD, rd_rs2, rs1s,
> - GET_C_LD_IMM(ctx->opcode));
> + GET_C_LD_IMM(opcode));
> #else
> /* C.FLW (RV32) -> flw rd', offset[6:2](rs1')*/
> gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
> -GET_C_LW_IMM(ctx->opcode));
> +GET_C_LW_IMM(opcode));
> #endif
> break;
> case 7:
> #if defined(TARGET_RISCV64)
> /* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
> gen_store_c(ctx, OPC_RISC_SD, rs1s, rd_rs2,
> - GET_C_LD_IMM(ctx->opcode));
> + GET_C_LD_IMM(opcode));
> #else
> /* C.FSW (RV32) -> fsw rs2', offset[6:2](rs1')*/
> gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
> - GET_C_LW_IMM(ctx->opcode));
> + GET_C_LW_IMM(opcode));
> #endif
> break;
> }
> }
>
> -static void decode_RV32_64C(DisasContext *ctx)
> +static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
> {
> -uint8_t op = extract32(ctx->opcode, 0, 2);
> +uint8_t op = extract16(opcode, 0, 2);
>
> switch (op) {
> case 0:
> -decode_RV32_64C0(ctx);
> +decode_RV32_64C0(ctx, opcode);
> break;
> }
> }
> @@ -709,22 +708,25 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
> /* Include the auto-generated decoder for 16 bit insn */
> #include "decode_insn16.inc.c"
>
> -static void decode_opc(DisasContext *ctx)
> +static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t
> opcode)
> {
> /* check for compressed insn */
> -if (extract32(ctx->opcode, 0, 2) != 3) {
> +
Re: [PATCH v2 11/19] plugins/core: add missing break in cb_to_tcg_flags
On 2/13/20 11:51 PM, Alex Bennée wrote:
> From: "Emilio G. Cota"
>
> Reported-by: Robert Henry
> Signed-off-by: Emilio G. Cota
> Signed-off-by: Alex Bennée
> Reviewed-by: Richard Henderson
> Message-Id: <20200105072940.32204-1-c...@braap.org>
Fixes: 54cb65d8588
Reviewed-by: Philippe Mathieu-Daudé
> Cc: qemu-sta...@nongnu.org
> ---
> plugins/core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/plugins/core.c b/plugins/core.c
> index 9e1b9e7a915..ed863011baf 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -286,6 +286,7 @@ static inline uint32_t cb_to_tcg_flags(enum
> qemu_plugin_cb_flags flags)
> switch (flags) {
> case QEMU_PLUGIN_CB_RW_REGS:
> ret = 0;
> +break;
> case QEMU_PLUGIN_CB_R_REGS:
> ret = TCG_CALL_NO_WG;
> break;
>
Re: [PATCH v2 18/19] tests/tcg: fix typo in configure.sh test for v8.3
On 2/13/20 11:51 PM, Alex Bennée wrote: > Although most people use the docker images this can trip up on > developer systems with actual valid cross-compilers! > Oops =) Fixes: bb516dfc5b3 Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: Alex Bennée > --- > tests/tcg/configure.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh > index 9eb6ba3b7ea..eaaaff6233a 100755 > --- a/tests/tcg/configure.sh > +++ b/tests/tcg/configure.sh > @@ -228,7 +228,7 @@ for target in $target_list; do > echo "CROSS_CC_HAS_SVE=y" >> $config_target_mak > fi > if do_compiler "$target_compiler" $target_compiler_cflags \ > - -march=-march=armv8.3-a -o $TMPE $TMPC; then > + -march=armv8.3-a -o $TMPE $TMPC; then > echo "CROSS_CC_HAS_ARMV8_3=y" >> $config_target_mak > fi > ;; >
Re: [PATCH v2 16/19] tests/tcg: give debug builds a little bit longer
On 2/13/20 11:51 PM, Alex Bennée wrote: > When combined with heavy plugins we occasionally hit the timeouts. > > Signed-off-by: Alex Bennée > --- > tests/tcg/Makefile.target | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target > index 3c7421a356e..b3cff3cad1a 100644 > --- a/tests/tcg/Makefile.target > +++ b/tests/tcg/Makefile.target > @@ -79,7 +79,7 @@ QEMU_OPTS= > > # If TCG debugging is enabled things are a lot slower > ifeq ($(CONFIG_DEBUG_TCG),y) > -TIMEOUT=45 > +TIMEOUT=60 > else > TIMEOUT=15 > endif > @@ -137,7 +137,7 @@ PLUGINS=$(notdir $(wildcard $(PLUGIN_DIR)/*.so)) > $(foreach p,$(PLUGINS), \ > $(foreach t,$(TESTS),\ > $(eval run-plugin-$(t)-with-$(p): $t $p) \ > - $(eval run-plugin-$(t)-with-$(p): TIMEOUT=30) \ > + $(eval run-plugin-$(t)-with-$(p): TIMEOUT=60) \ > $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p > endif > > Reviewed-by: Philippe Mathieu-Daudé
[PATCH] hw/net/i82596: Correct command bitmask (CID 1419392)
The command is 32-bit, but we are loading the 16 upper bits with
the 'get_uint16(s->scb + 2)' call.
Once shifted by 16, the command bits match the status bits:
- Command
Bit 31 ACK-CX Acknowledges that the CU completed an Action Command.
Bit 30 ACK-FR Acknowledges that the RU received a frame.
Bit 29 ACK-CNA Acknowledges that the Command Unit became not active.
Bit 28 ACK-RNR Acknowledges that the Receive Unit became not ready.
- Status
Bit 15 CX The CU finished executing a command with its I(interrupt) bit
set.
Bit 14 FR The RU finished receiving a frame.
Bit 13 CNA The Command Unit left the Active state.
Bit 12 RNR The Receive Unit left the Ready state.
Add the SCB_COMMAND_ACK_MASK definition to simplify the code.
This fixes Coverity 1419392 (CONSTANT_EXPRESSION_RESULT):
/hw/net/i82596.c: 352 in examine_scb()
346 cuc = (command >> 8) & 0x7;
347 ruc = (command >> 4) & 0x7;
348 DBG(printf("MAIN COMMAND %04x cuc %02x ruc %02x\n", command,
cuc, ruc));
349 /* and clear the scb command word */
350 set_uint16(s->scb + 2, 0);
351
>>> CID 1419392:(CONSTANT_EXPRESSION_RESULT)
>>> "command & (2147483648UL /* 1UL << 31 */)" is always 0 regardless of
the values of its operands. This occurs as the logical operand of "if".
352 if (command & BIT(31)) /* ACK-CX */
353 s->scb_status &= ~SCB_STATUS_CX;
>>> CID 1419392:(CONSTANT_EXPRESSION_RESULT)
>>> "command & (1073741824UL /* 1UL << 30 */)" is always 0 regardless of
the values of its operands. This occurs as the logical operand of "if".
354 if (command & BIT(30)) /*ACK-FR */
355 s->scb_status &= ~SCB_STATUS_FR;
>>> CID 1419392:(CONSTANT_EXPRESSION_RESULT)
>>> "command & (536870912UL /* 1UL << 29 */)" is always 0 regardless of
the values of its operands. This occurs as the logical operand of "if".
356 if (command & BIT(29)) /*ACK-CNA */
357 s->scb_status &= ~SCB_STATUS_CNA;
>>> CID 1419392:(CONSTANT_EXPRESSION_RESULT)
>>> "command & (268435456UL /* 1UL << 28 */)" is always 0 regardless of
the values of its operands. This occurs as the logical operand of "if".
358 if (command & BIT(28)) /*ACK-RNR */
359 s->scb_status &= ~SCB_STATUS_RNR;
Fixes: Covertiy CID 1419392 (commit 376b851909)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/i82596.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index 3a0e1ec4c0..b7c2458a96 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -43,6 +43,9 @@
#define SCB_STATUS_CNA 0x2000 /* CU left active state */
#define SCB_STATUS_RNR 0x1000 /* RU left active state */
+#define SCB_COMMAND_ACK_MASK \
+(SCB_STATUS_CX | SCB_STATUS_FR | SCB_STATUS_CNA | SCB_STATUS_RNR)
+
#define CU_IDLE 0
#define CU_SUSPENDED1
#define CU_ACTIVE 2
@@ -349,14 +352,7 @@ static void examine_scb(I82596State *s)
/* and clear the scb command word */
set_uint16(s->scb + 2, 0);
-if (command & BIT(31)) /* ACK-CX */
-s->scb_status &= ~SCB_STATUS_CX;
-if (command & BIT(30)) /*ACK-FR */
-s->scb_status &= ~SCB_STATUS_FR;
-if (command & BIT(29)) /*ACK-CNA */
-s->scb_status &= ~SCB_STATUS_CNA;
-if (command & BIT(28)) /*ACK-RNR */
-s->scb_status &= ~SCB_STATUS_RNR;
+s->scb_status &= ~(command & SCB_COMMAND_ACK_MASK);
switch (cuc) {
case 0: /* no change */
--
2.21.1
Re: [PATCH] target/ppc: Fix typo in comments
On Fri, Feb 14, 2020 at 12:57:34AM +0100, BALATON Zoltan wrote: > "Deferred" was misspelled as "differed" in some comments, correct this > typo, > > Signed-off-by: BALATON Zoltan Applied to ppc-for-5.0, thanks. > --- > target/ppc/fpu_helper.c| 2 +- > target/ppc/translate/fp-impl.inc.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index dc383242f7..5182764df3 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -293,7 +293,7 @@ static void float_invalid_op_vxvc(CPUPPCState *env, bool > set_fpcc, > env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_VXVC; > /* Update the floating-point enabled exception summary */ > env->fpscr |= FP_FEX; > -/* Exception is differed */ > +/* Exception is deferred */ > } > } > > diff --git a/target/ppc/translate/fp-impl.inc.c > b/target/ppc/translate/fp-impl.inc.c > index d8e27bf4d5..9f7868ee28 100644 > --- a/target/ppc/translate/fp-impl.inc.c > +++ b/target/ppc/translate/fp-impl.inc.c > @@ -781,7 +781,7 @@ static void gen_mtfsb1(DisasContext *ctx) > tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > -/* We can raise a differed exception */ > +/* We can raise a deferred exception */ > gen_helper_float_check_status(cpu_env); > } > > @@ -817,7 +817,7 @@ static void gen_mtfsf(DisasContext *ctx) > tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > -/* We can raise a differed exception */ > +/* We can raise a deferred exception */ > gen_helper_float_check_status(cpu_env); > tcg_temp_free_i64(t1); > } > @@ -850,7 +850,7 @@ static void gen_mtfsfi(DisasContext *ctx) > tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > -/* We can raise a differed exception */ > +/* We can raise a deferred exception */ > gen_helper_float_check_status(cpu_env); > } > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH] target/ppc: Fix typo in comments
On 2/14/20 12:57 AM, BALATON Zoltan wrote: > "Deferred" was misspelled as "differed" in some comments, correct this > typo, > Fixes: 7c58044c Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: BALATON Zoltan > --- > target/ppc/fpu_helper.c| 2 +- > target/ppc/translate/fp-impl.inc.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index dc383242f7..5182764df3 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -293,7 +293,7 @@ static void float_invalid_op_vxvc(CPUPPCState *env, bool > set_fpcc, > env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_VXVC; > /* Update the floating-point enabled exception summary */ > env->fpscr |= FP_FEX; > -/* Exception is differed */ > +/* Exception is deferred */ > } > } > > diff --git a/target/ppc/translate/fp-impl.inc.c > b/target/ppc/translate/fp-impl.inc.c > index d8e27bf4d5..9f7868ee28 100644 > --- a/target/ppc/translate/fp-impl.inc.c > +++ b/target/ppc/translate/fp-impl.inc.c > @@ -781,7 +781,7 @@ static void gen_mtfsb1(DisasContext *ctx) > tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > -/* We can raise a differed exception */ > +/* We can raise a deferred exception */ > gen_helper_float_check_status(cpu_env); > } > > @@ -817,7 +817,7 @@ static void gen_mtfsf(DisasContext *ctx) > tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > -/* We can raise a differed exception */ > +/* We can raise a deferred exception */ > gen_helper_float_check_status(cpu_env); > tcg_temp_free_i64(t1); > } > @@ -850,7 +850,7 @@ static void gen_mtfsfi(DisasContext *ctx) > tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > -/* We can raise a differed exception */ > +/* We can raise a deferred exception */ > gen_helper_float_check_status(cpu_env); > } > >
[PATCH 3/5] hw/display/artist: Delay some variables initialization
We want to have an early exit path. Delay some initializations
before the variables are used.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/display/artist.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 47f0e9f0bc..97c811b35e 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -557,90 +557,90 @@ static void fill_window(ARTISTState *s, int startx, int
starty,
static void draw_line(ARTISTState *s, int x1, int y1, int x2, int y2,
bool update_start, int skip_pix, int max_pix)
{
struct vram_buffer *buf;
-uint8_t color = artist_get_color(s);
+uint8_t color;
int dx, dy, t, e, x, y, incy, diago, horiz;
bool c1;
uint8_t *p;
trace_artist_draw_line(x1, y1, x2, y2);
if (update_start) {
s->vram_start = (x2 << 16) | y2;
}
-buf = >vram_buffer[ARTIST_BUFFER_AP];
-
-c1 = false;
-
if (x2 > x1) {
dx = x2 - x1;
} else {
dx = x1 - x2;
}
if (y2 > y1) {
dy = y2 - y1;
} else {
dy = y1 - y2;
}
+
+c1 = false;
if (dy > dx) {
t = y2;
y2 = x2;
x2 = t;
t = y1;
y1 = x1;
x1 = t;
t = dx;
dx = dy;
dy = t;
c1 = true;
}
if (x1 > x2) {
t = y2;
y2 = y1;
y1 = t;
t = x1;
x1 = x2;
x2 = t;
}
horiz = dy << 1;
diago = (dy - dx) << 1;
e = (dy << 1) - dx;
if (y1 <= y2) {
incy = 1;
} else {
incy = -1;
}
x = x1;
y = y1;
+color = artist_get_color(s);
+buf = >vram_buffer[ARTIST_BUFFER_AP];
do {
if (c1) {
p = buf->data + x * s->width + y;
} else {
p = buf->data + y * s->width + x;
}
if (skip_pix > 0) {
skip_pix--;
} else {
artist_rop8(s, p, color);
}
if (e > 0) {
artist_invalidate_lines(buf, y, 1);
y += incy;
e += diago;
} else {
e += horiz;
}
x++;
} while (x <= x2 && (max_pix == -1 || --max_pix > 0));
}
--
2.21.1
[RFC PATCH 4/5] hw/display/artist: Avoid drawing line when nothing to display
Signed-off-by: Philippe Mathieu-Daudé
---
RFC because untested =)
---
hw/display/artist.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 97c811b35e..5492079116 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -557,90 +557,93 @@ static void fill_window(ARTISTState *s, int startx, int
starty,
static void draw_line(ARTISTState *s, int x1, int y1, int x2, int y2,
bool update_start, int skip_pix, int max_pix)
{
struct vram_buffer *buf;
uint8_t color;
int dx, dy, t, e, x, y, incy, diago, horiz;
bool c1;
uint8_t *p;
trace_artist_draw_line(x1, y1, x2, y2);
if (update_start) {
s->vram_start = (x2 << 16) | y2;
}
if (x2 > x1) {
dx = x2 - x1;
} else {
dx = x1 - x2;
}
if (y2 > y1) {
dy = y2 - y1;
} else {
dy = y1 - y2;
}
+if (!dx || !dy) {
+return;
+}
c1 = false;
if (dy > dx) {
t = y2;
y2 = x2;
x2 = t;
t = y1;
y1 = x1;
x1 = t;
t = dx;
dx = dy;
dy = t;
c1 = true;
}
if (x1 > x2) {
t = y2;
y2 = y1;
y1 = t;
t = x1;
x1 = x2;
x2 = t;
}
horiz = dy << 1;
diago = (dy - dx) << 1;
e = (dy << 1) - dx;
if (y1 <= y2) {
incy = 1;
} else {
incy = -1;
}
x = x1;
y = y1;
color = artist_get_color(s);
buf = >vram_buffer[ARTIST_BUFFER_AP];
do {
if (c1) {
p = buf->data + x * s->width + y;
} else {
p = buf->data + y * s->width + x;
}
if (skip_pix > 0) {
skip_pix--;
} else {
artist_rop8(s, p, color);
}
if (e > 0) {
artist_invalidate_lines(buf, y, 1);
y += incy;
e += diago;
} else {
e += horiz;
}
x++;
} while (x <= x2 && (max_pix == -1 || --max_pix > 0));
}
--
2.21.1
[PATCH 5/5] hw/display/artist: Remove dead code (CID 1419388 & 1419389)
Coverity reports:
*** CID 1419388: Control flow issues (DEADCODE)
/hw/display/artist.c: 739 in draw_line_xy()
733 if (endy < 0) {
734 endy = 0;
735 }
736
737
738 if (endx < 0) {
>>> CID 1419388: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "return;".
739 return;
740 }
741
742 if (endy < 0) {
743 return;
744 }
*** CID 1419389: Control flow issues (DEADCODE)
/hw/display/artist.c: 743 in draw_line_xy()
737
738 if (endx < 0) {
739 return;
740 }
741
742 if (endy < 0) {
>>> CID 1419389: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "return;".
743 return;
744 }
745
746 trace_artist_draw_line(startx, starty, endx, endy);
747 draw_line(s, startx, starty, endx, endy, false, -1, -1);
748 }
Fixes: Covertiy CID 1419388 and 1419389 (commit 4765384ce33)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/display/artist.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 5492079116..753dbb9a77 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -690,59 +690,50 @@ static void draw_line_size(ARTISTState *s, bool
update_start)
static void draw_line_xy(ARTISTState *s, bool update_start)
{
int startx = artist_get_x(s->vram_start);
int starty = artist_get_y(s->vram_start);
int sizex = artist_get_x(s->blockmove_size);
int sizey = artist_get_y(s->blockmove_size);
int linexy = s->line_xy >> 16;
int endx, endy;
endx = startx;
endy = starty;
if (sizex > 0) {
endx = startx + linexy;
}
if (sizex < 0) {
endx = startx;
startx -= linexy;
}
if (sizey > 0) {
endy = starty + linexy;
}
if (sizey < 0) {
endy = starty;
starty -= linexy;
}
if (startx < 0) {
startx = 0;
}
if (endx < 0) {
endx = 0;
}
if (starty < 0) {
starty = 0;
}
if (endy < 0) {
endy = 0;
}
-
-if (endx < 0) {
-return;
-}
-
-if (endy < 0) {
-return;
-}
-
draw_line(s, startx, starty, endx, endy, false, -1, -1);
}
--
2.21.1
[PATCH 2/5] hw/display/artist: Remove pointless initialization
We are initializating incy inconditionally:
if (y1 <= y2) {
incy = 1;
} else {
incy = -1;
}
Signed-off-by: Philippe Mathieu-Daudé
---
hw/display/artist.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index abacb0e27d..47f0e9f0bc 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -557,91 +557,90 @@ static void fill_window(ARTISTState *s, int startx, int
starty,
static void draw_line(ARTISTState *s, int x1, int y1, int x2, int y2,
bool update_start, int skip_pix, int max_pix)
{
struct vram_buffer *buf;
uint8_t color = artist_get_color(s);
int dx, dy, t, e, x, y, incy, diago, horiz;
bool c1;
uint8_t *p;
trace_artist_draw_line(x1, y1, x2, y2);
if (update_start) {
s->vram_start = (x2 << 16) | y2;
}
buf = >vram_buffer[ARTIST_BUFFER_AP];
c1 = false;
-incy = 1;
if (x2 > x1) {
dx = x2 - x1;
} else {
dx = x1 - x2;
}
if (y2 > y1) {
dy = y2 - y1;
} else {
dy = y1 - y2;
}
if (dy > dx) {
t = y2;
y2 = x2;
x2 = t;
t = y1;
y1 = x1;
x1 = t;
t = dx;
dx = dy;
dy = t;
c1 = true;
}
if (x1 > x2) {
t = y2;
y2 = y1;
y1 = t;
t = x1;
x1 = x2;
x2 = t;
}
horiz = dy << 1;
diago = (dy - dx) << 1;
e = (dy << 1) - dx;
if (y1 <= y2) {
incy = 1;
} else {
incy = -1;
}
x = x1;
y = y1;
do {
if (c1) {
p = buf->data + x * s->width + y;
} else {
p = buf->data + y * s->width + x;
}
if (skip_pix > 0) {
skip_pix--;
} else {
artist_rop8(s, p, color);
}
if (e > 0) {
artist_invalidate_lines(buf, y, 1);
y += incy;
e += diago;
} else {
e += horiz;
}
x++;
} while (x <= x2 && (max_pix == -1 || --max_pix > 0));
}
--
2.21.1
[PATCH 0/5] hw/display/artist: Fix Coverity 1419388 & 1419389
Another easy Coverity fix. Philippe Mathieu-Daudé (5): hw/display/artist: Move trace event to draw_line() hw/display/artist: Remove pointless initialization hw/display/artist: Delay some variables initialization hw/display/artist: Avoid drawing line when nothing to display hw/display/artist: Remove dead code (CID 1419388 & 1419389) hw/display/artist.c | 29 + 1 file changed, 9 insertions(+), 20 deletions(-) -- 2.21.1
[PATCH 1/5] hw/display/artist: Move trace event to draw_line()
Instead of emitting the trace event before each call to
draw_line(), call it once at draw_line() entrance.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/display/artist.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 65be9e3554..abacb0e27d 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -557,90 +557,91 @@ static void fill_window(ARTISTState *s, int startx, int
starty,
static void draw_line(ARTISTState *s, int x1, int y1, int x2, int y2,
bool update_start, int skip_pix, int max_pix)
{
struct vram_buffer *buf;
uint8_t color = artist_get_color(s);
int dx, dy, t, e, x, y, incy, diago, horiz;
bool c1;
uint8_t *p;
+trace_artist_draw_line(x1, y1, x2, y2);
if (update_start) {
s->vram_start = (x2 << 16) | y2;
}
buf = >vram_buffer[ARTIST_BUFFER_AP];
c1 = false;
incy = 1;
if (x2 > x1) {
dx = x2 - x1;
} else {
dx = x1 - x2;
}
if (y2 > y1) {
dy = y2 - y1;
} else {
dy = y1 - y2;
}
if (dy > dx) {
t = y2;
y2 = x2;
x2 = t;
t = y1;
y1 = x1;
x1 = t;
t = dx;
dx = dy;
dy = t;
c1 = true;
}
if (x1 > x2) {
t = y2;
y2 = y1;
y1 = t;
t = x1;
x1 = x2;
x2 = t;
}
horiz = dy << 1;
diago = (dy - dx) << 1;
e = (dy << 1) - dx;
if (y1 <= y2) {
incy = 1;
} else {
incy = -1;
}
x = x1;
y = y1;
do {
if (c1) {
p = buf->data + x * s->width + y;
} else {
p = buf->data + y * s->width + x;
}
if (skip_pix > 0) {
skip_pix--;
} else {
artist_rop8(s, p, color);
}
if (e > 0) {
artist_invalidate_lines(buf, y, 1);
y += incy;
e += diago;
} else {
e += horiz;
}
x++;
} while (x <= x2 && (max_pix == -1 || --max_pix > 0));
}
@@ -648,13 +649,12 @@ static void draw_line(ARTISTState *s, int x1, int y1, int
x2, int y2,
static void draw_line_pattern_start(ARTISTState *s)
{
int startx = artist_get_x(s->vram_start);
int starty = artist_get_y(s->vram_start);
int endx = artist_get_x(s->blockmove_size);
int endy = artist_get_y(s->blockmove_size);
int pstart = s->line_pattern_start >> 16;
-trace_artist_draw_line(startx, starty, endx, endy);
draw_line(s, startx, starty, endx, endy, false, -1, pstart);
s->line_pattern_skip = pstart;
}
@@ -662,15 +662,14 @@ static void draw_line_pattern_start(ARTISTState *s)
static void draw_line_pattern_next(ARTISTState *s)
{
int startx = artist_get_x(s->vram_start);
int starty = artist_get_y(s->vram_start);
int endx = artist_get_x(s->blockmove_size);
int endy = artist_get_y(s->blockmove_size);
int line_xy = s->line_xy >> 16;
-trace_artist_draw_line(startx, starty, endx, endy);
draw_line(s, startx, starty, endx, endy, false, s->line_pattern_skip,
s->line_pattern_skip + line_xy);
s->line_pattern_skip += line_xy;
s->image_bitmap_op ^= 2;
}
@@ -678,84 +677,81 @@ static void draw_line_pattern_next(ARTISTState *s)
static void draw_line_size(ARTISTState *s, bool update_start)
{
int startx = artist_get_x(s->vram_start);
int starty = artist_get_y(s->vram_start);
int endx = artist_get_x(s->line_size);
int endy = artist_get_y(s->line_size);
-trace_artist_draw_line(startx, starty, endx, endy);
draw_line(s, startx, starty, endx, endy, update_start, -1, -1);
}
static void draw_line_xy(ARTISTState *s, bool update_start)
{
int startx = artist_get_x(s->vram_start);
int starty = artist_get_y(s->vram_start);
int sizex = artist_get_x(s->blockmove_size);
int sizey = artist_get_y(s->blockmove_size);
int linexy = s->line_xy >> 16;
int endx, endy;
endx = startx;
endy = starty;
if (sizex > 0) {
endx = startx + linexy;
}
if (sizex < 0) {
endx = startx;
startx -= linexy;
}
if (sizey > 0) {
endy = starty + linexy;
}
if (sizey < 0) {
endy = starty;
starty -= linexy;
}
if (startx < 0) {
startx = 0;
}
if (endx < 0) {
endx = 0;
}
if (starty < 0) {
starty = 0;
}
if (endy < 0) {
endy = 0;
}
if (endx < 0) {
return;
}
if (endy < 0) {
return;
}
-trace_artist_draw_line(startx, starty, endx, endy);
draw_line(s, startx, starty, endx, endy, false, -1, -1);
}
static void draw_line_end(ARTISTState *s, bool update_start)
{
int startx =
Re: VW ELF loader
On 13/02/2020 21:17, Paolo Bonzini wrote: > On 13/02/20 02:43, Alexey Kardashevskiy wrote: >> >> Ok. So, I have made a small firmware which does OF CI, loads GRUB and >> instantiates RTAS: >> https://github.com/aik/of1275 >> Quite raw but gives the idea. >> >> It does not contain drivers and still relies on QEMU to hook an OF path >> to a backend. Is this a showstopper and without drivers it is no go? Thanks, > > Yes, it's really the drivers. Something like netboot wouldn't work for > example. > > I don't have a problem with relying on QEMU for opening and closing OF > paths, but I really believe that read/write on ihandles should be done > within the firmware and not QEMU. Moving read/write to the firmware is not a problem but there is a little mix up here :) An ihandle is open from a path and nothing there suggests drivers, it is up to the ihandle's "read" method what happens next. If we do PCI drivers in the firmware, then the entire ihandle (== "opened instance of a phandle") business goes to the firmware and we are slowly bringing the existing mess back again. -- Alexey
[PATCH] target/ppc: Fix typo in comments
"Deferred" was misspelled as "differed" in some comments, correct this typo, Signed-off-by: BALATON Zoltan --- target/ppc/fpu_helper.c| 2 +- target/ppc/translate/fp-impl.inc.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index dc383242f7..5182764df3 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -293,7 +293,7 @@ static void float_invalid_op_vxvc(CPUPPCState *env, bool set_fpcc, env->error_code = POWERPC_EXCP_FP | POWERPC_EXCP_FP_VXVC; /* Update the floating-point enabled exception summary */ env->fpscr |= FP_FEX; -/* Exception is differed */ +/* Exception is deferred */ } } diff --git a/target/ppc/translate/fp-impl.inc.c b/target/ppc/translate/fp-impl.inc.c index d8e27bf4d5..9f7868ee28 100644 --- a/target/ppc/translate/fp-impl.inc.c +++ b/target/ppc/translate/fp-impl.inc.c @@ -781,7 +781,7 @@ static void gen_mtfsb1(DisasContext *ctx) tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); } -/* We can raise a differed exception */ +/* We can raise a deferred exception */ gen_helper_float_check_status(cpu_env); } @@ -817,7 +817,7 @@ static void gen_mtfsf(DisasContext *ctx) tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); } -/* We can raise a differed exception */ +/* We can raise a deferred exception */ gen_helper_float_check_status(cpu_env); tcg_temp_free_i64(t1); } @@ -850,7 +850,7 @@ static void gen_mtfsfi(DisasContext *ctx) tcg_gen_trunc_tl_i32(cpu_crf[1], cpu_fpscr); tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); } -/* We can raise a differed exception */ +/* We can raise a deferred exception */ gen_helper_float_check_status(cpu_env); } -- 2.21.1
[PATCH 2/4] hw/hppa/dino: Fix reg800_keep_bits[] overrun (CID 1419393 & 1419394)
Coverity reports: *** CID 1419393: Memory - corruptions (OVERRUN) /hw/hppa/dino.c: 363 in dino_chip_write_with_attrs() 357 /* These registers are read-only. */ 358 break; 359 360 case DINO_GMASK ... DINO_TLTIM: 361 i = (addr - DINO_GMASK) / 4; 362 val &= reg800_keep_bits[i]; >>> CID 1419393: Memory - corruptions (OVERRUN) >>> Overrunning array "s->reg800" of 12 4-byte elements at element index 12 (byte offset 48) using index "i" (which evaluates to 12). 363 s->reg800[i] = val; 364 break; 365 366 default: 367 /* Controlled by dino_chip_mem_valid above. */ 368 g_assert_not_reached(); and: *** CID 1419394: Memory - illegal accesses (OVERRUN) /hw/hppa/dino.c: 362 in dino_chip_write_with_attrs() 356 case DINO_IRR1: 357 /* These registers are read-only. */ 358 break; 359 360 case DINO_GMASK ... DINO_TLTIM: 361 i = (addr - DINO_GMASK) / 4; >>> CID 1419394: Memory - illegal accesses (OVERRUN) >>> Overrunning array "reg800_keep_bits" of 12 4-byte elements at element index 12 (byte offset 48) using index "i" (which evaluates to 12). 362 val &= reg800_keep_bits[i]; 363 s->reg800[i] = val; 364 break; 365 366 default: 367 /* Controlled by dino_chip_mem_valid above. */ Indeed the array should contain 13 entries, the undocumented register 0x82c is missing. Fix by increasing the array size and adding the missing register. CID 1419393 can be verified with: $ echo x 0xfff80830 | hppa-softmmu/qemu-system-hppa -S -monitor stdio -display none QEMU 4.2.50 monitor - type 'help' for more information (qemu) x 0xfff80830 qemu/hw/hppa/dino.c:267:15: runtime error: index 12 out of bounds for type 'uint32_t [12]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/phil/source/qemu/hw/hppa/dino.c:267:15 in fff80830: 0x and CID 1419394 with: $ echo writeb 0xfff80830 0x69 \ | hppa-softmmu/qemu-system-hppa -S -accel qtest -qtest stdio -display none [I 1581634452.654113] OPENED [R +4.105415] writeb 0xfff80830 0x69 qemu/hw/hppa/dino.c:362:16: runtime error: index 12 out of bounds for type 'const uint32_t [12]' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior qemu/hw/hppa/dino.c:362:16 in = ==29607==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5577dae32f30 at pc 0x5577d93f2463 bp 0x7ffd97ea11b0 sp 0x7ffd97ea11a8 READ of size 4 at 0x5577dae32f30 thread T0 #0 0x5577d93f2462 in dino_chip_write_with_attrs qemu/hw/hppa/dino.c:362:16 #1 0x5577d9025664 in memory_region_write_with_attrs_accessor qemu/memory.c:503:12 #2 0x5577d9024920 in access_with_adjusted_size qemu/memory.c:539:18 #3 0x5577d9023608 in memory_region_dispatch_write qemu/memory.c:1482:13 #4 0x5577d8e3177a in flatview_write_continue qemu/exec.c:3166:23 #5 0x5577d8e20357 in flatview_write qemu/exec.c:3206:14 #6 0x5577d8e1fef4 in address_space_write qemu/exec.c:3296:18 #7 0x5577d8e20693 in address_space_rw qemu/exec.c:3306:16 #8 0x5577d9011595 in qtest_process_command qemu/qtest.c:432:13 #9 0x5577d900d19f in qtest_process_inbuf qemu/qtest.c:705:9 #10 0x5577d900ca22 in qtest_read qemu/qtest.c:717:5 #11 0x5577da8c4254 in qemu_chr_be_write_impl qemu/chardev/char.c:183:9 #12 0x5577da8c430c in qemu_chr_be_write qemu/chardev/char.c:195:9 #13 0x5577da8cf587 in fd_chr_read qemu/chardev/char-fd.c:68:9 #14 0x5577da9836cd in qio_channel_fd_source_dispatch qemu/io/channel-watch.c:84:12 #15 0x7faf44509ecc in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x4fecc) #16 0x5577dab75f96 in glib_pollfds_poll qemu/util/main-loop.c:219:9 #17 0x5577dab74797 in os_host_main_loop_wait qemu/util/main-loop.c:242:5 #18 0x5577dab7435a in main_loop_wait qemu/util/main-loop.c:518:11 #19 0x5577d9514eb3 in main_loop qemu/vl.c:1682:9 #20 0x5577d950699d in main qemu/vl.c:4450:5 #21 0x7faf41a87f42 in __libc_start_main (/lib64/libc.so.6+0x23f42) #22 0x5577d8cd4d4d in _start (qemu/build/sanitizer/hppa-softmmu/qemu-system-hppa+0x1256d4d) 0x5577dae32f30 is located 0 bytes to the right of global variable 'reg800_keep_bits' defined in 'qemu/hw/hppa/dino.c:87:23' (0x5577dae32f00) of size 48 SUMMARY: AddressSanitizer: global-buffer-overflow qemu/hw/hppa/dino.c:362:16 in dino_chip_write_with_attrs Shadow bytes around the buggy address: 0x0aaf7b5be590: 00 f9 f9 f9 f9 f9 f9 f9 00 02 f9 f9 f9 f9 f9 f9 0x0aaf7b5be5a0: 07 f9 f9 f9 f9 f9 f9 f9 07 f9 f9 f9 f9 f9 f9 f9 0x0aaf7b5be5b0: 07 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00 0x0aaf7b5be5c0: 00 00 00 02 f9 f9 f9 f9 00 00 00 00 00 00 00 00
[PATCH 1/4] hw/hppa/dino: Add comments with register name
Add a comment with the name of each register in the 0x800-0x833 range.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/hppa/dino.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
index 9797a7f0d9..c237ad3b1b 100644
--- a/hw/hppa/dino.c
+++ b/hw/hppa/dino.c
@@ -85,18 +85,18 @@
#define DINO800_REGS ((DINO_TLTIM - DINO_GMASK) / 4)
static const uint32_t reg800_keep_bits[DINO800_REGS] = {
-MAKE_64BIT_MASK(0, 1),
-MAKE_64BIT_MASK(0, 7),
-MAKE_64BIT_MASK(0, 7),
-MAKE_64BIT_MASK(0, 8),
-MAKE_64BIT_MASK(0, 7),
-MAKE_64BIT_MASK(0, 9),
-MAKE_64BIT_MASK(0, 32),
-MAKE_64BIT_MASK(0, 8),
-MAKE_64BIT_MASK(0, 30),
-MAKE_64BIT_MASK(0, 25),
-MAKE_64BIT_MASK(0, 22),
-MAKE_64BIT_MASK(0, 9),
+MAKE_64BIT_MASK(0, 1), /* GMASK */
+MAKE_64BIT_MASK(0, 7), /* PAMR */
+MAKE_64BIT_MASK(0, 7), /* PAPR */
+MAKE_64BIT_MASK(0, 8), /* DAMODE */
+MAKE_64BIT_MASK(0, 7), /* PCICMD */
+MAKE_64BIT_MASK(0, 9), /* PCISTS */
+MAKE_64BIT_MASK(0, 32), /* Undefined */
+MAKE_64BIT_MASK(0, 8), /* MLTIM */
+MAKE_64BIT_MASK(0, 30), /* BRDG_FEAT */
+MAKE_64BIT_MASK(0, 25), /* PCIROR */
+MAKE_64BIT_MASK(0, 22), /* PCIWOR */
+MAKE_64BIT_MASK(0, 9), /* TLTIM */
};
typedef struct DinoState {
--
2.21.1
[RFC PATCH 4/4] hw/hppa/dino: Do not accept accesses to registers 0x818 and 0x82c
Register 0x818 is documented as 'undefined', and register 0x82c is not documented. Refuse their access. Signed-off-by: Philippe Mathieu-Daudé --- hw/hppa/dino.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c index be799aad43..2b1b38c58a 100644 --- a/hw/hppa/dino.c +++ b/hw/hppa/dino.c @@ -181,7 +181,9 @@ static bool dino_chip_mem_valid(void *opaque, hwaddr addr, case DINO_IO_ADDR_EN: case DINO_PCI_IO_DATA: case DINO_TOC_ADDR: -case DINO_GMASK ... DINO_TLTIM: +case DINO_GMASK ... DINO_PCISTS: +case DINO_MLTIM ... DINO_PCIWOR: +case DINO_TLTIM: ret = true; break; case DINO_PCI_IO_DATA + 2: -- 2.21.1
[RFC PATCH 3/4] hw/hppa/dino: Fix PCIROR register access bitmask
Only 24 bits of the PCIROR register are documented
(see pp. 37 of datasheet referenced in this file header).
Signed-off-by: Philippe Mathieu-Daudé
---
hw/hppa/dino.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
index 8868e31793..be799aad43 100644
--- a/hw/hppa/dino.c
+++ b/hw/hppa/dino.c
@@ -94,7 +94,7 @@ static const uint32_t reg800_keep_bits[DINO800_REGS] = {
MAKE_64BIT_MASK(0, 32), /* Undefined */
MAKE_64BIT_MASK(0, 8), /* MLTIM */
MAKE_64BIT_MASK(0, 30), /* BRDG_FEAT */
-MAKE_64BIT_MASK(0, 25), /* PCIROR */
+MAKE_64BIT_MASK(0, 24), /* PCIROR */
MAKE_64BIT_MASK(0, 22), /* PCIWOR */
MAKE_64BIT_MASK(0, 32), /* Undocumented */
MAKE_64BIT_MASK(0, 9), /* TLTIM */
--
2.21.1
[PATCH 0/4] hw/hppa/dino: Fix Coverity 1419393 & 1419394
Easy fix for the overrun reported by Coverity. Last 2 patches are RFC because I haven't tested them, I simply took note while reviewing the datasheet (I also checked the errata). Philippe Mathieu-Daudé (4): hw/hppa/dino: Add comments with register name hw/hppa/dino: Fix reg800_keep_bits[] overrun (CID 1419393 & 1419394) hw/hppa/dino: Fix bitmask for the PCIROR register hw/hppa/dino: Do not accept accesses to registers 0x818 and 0x82c hw/hppa/dino.c | 31 +-- 1 file changed, 17 insertions(+), 14 deletions(-) -- 2.21.1
Re: [PATCH v12 Kernel 4/7] vfio iommu: Implementation of ioctl to for dirty pages tracking.
On Fri, 14 Feb 2020 01:41:35 +0530
Kirti Wankhede wrote:
>
>
>
> +static int vfio_iova_dirty_bitmap(struct vfio_iommu *iommu, dma_addr_t
> iova,
> + size_t size, uint64_t pgsize,
> + unsigned char __user *bitmap)
> +{
> +struct vfio_dma *dma;
> +dma_addr_t i = iova, iova_limit;
> +unsigned int bsize, nbits = 0, l = 0;
> +unsigned long pgshift = __ffs(pgsize);
> +
> +while ((dma = vfio_find_dma(iommu, i, pgsize))) {
> +int ret, j;
> +unsigned int npages = 0, shift = 0;
> +unsigned char temp = 0;
> +
> +/* mark all pages dirty if all pages are pinned and
> mapped. */
> +if (dma->iommu_mapped) {
> +iova_limit = min(dma->iova + dma->size, iova +
> size);
> +npages = iova_limit/pgsize;
> +bitmap_set(dma->bitmap, 0, npages);
> >>>
> >>> npages is derived from iova_limit, which is the number of bits to set
> >>> dirty relative to the first requested iova, not iova zero, ie. the set
> >>> of dirty bits is offset from those requested unless iova == dma->iova.
> >>>
> >>
> >> Right, fixing.
> >>
> >>> Also I hope dma->bitmap was actually allocated. Not only does the
> >>> START error path potentially leave dirty tracking enabled without all
> >>> the bitmap allocated, when does the bitmap get allocated for a new
> >>> vfio_dma when dirty tracking is enabled? Seems it only occurs if a
> >>> vpfn gets marked dirty.
> >>>
> >>
> >> Right.
> >>
> >> Fixing error paths.
> >>
> >>
> +} else if (dma->bitmap) {
> +struct rb_node *n = rb_first(>pfn_list);
> +bool found = false;
> +
> +for (; n; n = rb_next(n)) {
> +struct vfio_pfn *vpfn = rb_entry(n,
> +struct vfio_pfn, node);
> +if (vpfn->iova >= i) {
> +found = true;
> +break;
> +}
> +}
> +
> +if (!found) {
> +i += dma->size;
> +continue;
> +}
> +
> +for (; n; n = rb_next(n)) {
> +unsigned int s;
> +struct vfio_pfn *vpfn = rb_entry(n,
> +struct vfio_pfn, node);
> +
> +if (vpfn->iova >= iova + size)
> +break;
> +
> +s = (vpfn->iova - dma->iova) >> pgshift;
> +bitmap_set(dma->bitmap, s, 1);
> +
> +iova_limit = vpfn->iova + pgsize;
> +}
> +npages = iova_limit/pgsize;
> >>>
> >>> Isn't iova_limit potentially uninitialized here? For example, if our
> >>> vfio_dma covers {0,8192} and we ask for the bitmap of {0,4096} and
> >>> there's a vpfn at {4096,8192}. I think that means vpfn->iova >= i
> >>> (4096 >= 0), so we break with found = true, then we test 4096 >= 0 +
> >>> 4096 and break, and npages = /pgsize.
> >>>
> >>
> >> Right, Fixing it.
> >>
> +}
> +
> +bsize = dirty_bitmap_bytes(npages);
> +shift = nbits % BITS_PER_BYTE;
> +
> +if (npages && shift) {
> +l--;
> +if (!access_ok((void __user *)bitmap + l,
> +sizeof(unsigned char)))
> +return -EINVAL;
> +
> +ret = __get_user(temp, bitmap + l);
> >>>
> >>> I don't understand why we care to get the user's bitmap, are we trying
> >>> to leave whatever garbage they might have set in it and only also set
> >>> the dirty bits? That seems unnecessary.
> >>>
> >>
> >> Suppose dma mapped ranges are {start, size}:
> >> {0, 0xa000}, {0xa000, 0x1}
> >>
> >> Bitmap asked from 0 - 0x1. Say suppose all pages are dirty.
> >> Then in first iteration for dma {0,0xa000} there are 10 pages, so 10
> >> bits are set, put_user() happens for 2 bytes, (0011 b).
> >> In second iteration for dma {0xa000, 0x1} there are 6 pages and
> >> these bits should be appended to previous byte. So get_user() that
Re: [PATCH v2 00/19] testing and plugin updates
Patchew URL: https://patchew.org/QEMU/20200213225109.13120-1-alex.ben...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v2 00/19] testing and plugin updates Message-id: 20200213225109.13120-1-alex.ben...@linaro.org Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20200213225109.13120-1-alex.ben...@linaro.org -> patchew/20200213225109.13120-1-alex.ben...@linaro.org Switched to a new branch 'test' 3109a26 tests/tcg: take into account expected clashes pauth-4 bd088da tests/tcg: fix typo in configure.sh test for v8.3 45e0661 tcg: save vaddr temp for plugin usage 465e545 tests/tcg: give debug builds a little bit longer 1dc357e tests/plugins: make howvec clean-up after itself. fcb8562 target/riscv: progressively load the instruction during decode 2e6734c qemu/bitops.h: Add extract8 and extract16 21ff7e3 tests/plugin: prevent uninitialized warning 81f2039 plugins/core: add missing break in cb_to_tcg_flags 80651cf docs/devel: document query handle lifetimes 9dc3d6d tracing: only allow -trace to override -D if set eb2e0ad tests/iotests: be a little more forgiving on the size test 4d90ee2 travis.yml: single-thread build-tcg stages 3425df2 travis.yml: Fix Travis YAML configuration warnings 49b5310 travis.yml: Test the s390-ccw build, too 4ef30b9 tests/rcutorture: mild documenting refactor of update thread 23905a1 tests/rcutorture: better document locking of stats c3a20fc tests/rcutorture: update usage hint e31233b tests/tcg: include a skip runner for pauth3 with plugins === OUTPUT BEGIN === 1/19 Checking commit e31233ba5677 (tests/tcg: include a skip runner for pauth3 with plugins) 2/19 Checking commit c3a20fc36238 (tests/rcutorture: update usage hint) 3/19 Checking commit 23905a1b8851 (tests/rcutorture: better document locking of stats) 4/19 Checking commit 4ef30b9250b9 (tests/rcutorture: mild documenting refactor of update thread) 5/19 Checking commit 49b53100439e (travis.yml: Test the s390-ccw build, too) 6/19 Checking commit 3425df244fcd (travis.yml: Fix Travis YAML configuration warnings) 7/19 Checking commit 4d90ee26396b (travis.yml: single-thread build-tcg stages) 8/19 Checking commit eb2e0addbf96 (tests/iotests: be a little more forgiving on the size test) 9/19 Checking commit 9dc3d6d877be (tracing: only allow -trace to override -D if set) 10/19 Checking commit 80651cf4dd6a (docs/devel: document query handle lifetimes) 11/19 Checking commit 81f203940a48 (plugins/core: add missing break in cb_to_tcg_flags) 12/19 Checking commit 21ff7e396207 (tests/plugin: prevent uninitialized warning) 13/19 Checking commit 2e6734c5b2b1 (qemu/bitops.h: Add extract8 and extract16) 14/19 Checking commit fcb85625f96a (target/riscv: progressively load the instruction during decode) 15/19 Checking commit 1dc357e99457 (tests/plugins: make howvec clean-up after itself.) 16/19 Checking commit 465e5454070a (tests/tcg: give debug builds a little bit longer) 17/19 Checking commit 45e066189d4e (tcg: save vaddr temp for plugin usage) 18/19 Checking commit bd088dac07cc (tests/tcg: fix typo in configure.sh test for v8.3) 19/19 Checking commit 3109a268d5ae (tests/tcg: take into account expected clashes pauth-4) WARNING: Block comments use a leading /* on a separate line #60: FILE: tests/tcg/aarch64/pauth-4.c:25: +: /* out */ "=r"(x), "=r"(y) WARNING: Block comments use a leading /* on a separate line #61: FILE: tests/tcg/aarch64/pauth-4.c:26: +: /* in */ [in] "r" (in) ERROR: space prohibited before open square bracket '[' #61: FILE: tests/tcg/aarch64/pauth-4.c:26: +: /* in */ [in] "r" (in) WARNING: Block comments use a leading /* on a separate line #62: FILE: tests/tcg/aarch64/pauth-4.c:27: +: /* clobbers */); total: 1 errors, 3 warnings, 62 lines checked Patch 19/19 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200213225109.13120-1-alex.ben...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[PATCH v2 19/19] tests/tcg: take into account expected clashes pauth-4
Pointer authentication isn't perfect so measure the percentage of
failed checks. As we want to vary the pointer that is authenticated we
recurse down the stack.
Signed-off-by: Alex Bennée
---
tests/tcg/aarch64/pauth-4.c | 54 +
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/tests/tcg/aarch64/pauth-4.c b/tests/tcg/aarch64/pauth-4.c
index 1040e92aec3..24a639e36ca 100644
--- a/tests/tcg/aarch64/pauth-4.c
+++ b/tests/tcg/aarch64/pauth-4.c
@@ -1,25 +1,45 @@
#include
#include
+#include
+#include
+
+#define TESTS 1000
int main()
{
- uintptr_t x, y;
+int i, count = 0;
+float perc;
+void *base = malloc(TESTS);
+
+for (i = 0; i < TESTS; i++) {
+uintptr_t in, x, y;
+
+in = i + (uintptr_t) base;
+
+asm("mov %0, %[in]\n\t"
+"pacia %0, sp\n\t"/* sigill if pauth not supported */
+"eor %0, %0, #4\n\t" /* corrupt single bit */
+"mov %1, %0\n\t"
+"autia %1, sp\n\t"/* validate corrupted pointer */
+"xpaci %0\n\t"/* strip pac from corrupted pointer */
+: /* out */ "=r"(x), "=r"(y)
+: /* in */ [in] "r" (in)
+: /* clobbers */);
- asm("mov %0, lr\n\t"
- "pacia %0, sp\n\t"/* sigill if pauth not supported */
- "eor %0, %0, #4\n\t" /* corrupt single bit */
- "mov %1, %0\n\t"
- "autia %1, sp\n\t"/* validate corrupted pointer */
- "xpaci %0\n\t"/* strip pac from corrupted pointer */
- : "=r"(x), "=r"(y));
+/*
+ * Once stripped, the corrupted pointer is of the form 0x...wxyz.
+ * We expect the autia to indicate failure, producing a pointer of the
+ * form 0x000ewxyz. Use xpaci and != for the test, rather than
+ * extracting explicit bits from the top, because the location of the
+ * error code "e" depends on the configuration of virtual memory.
+ */
+if (x != y) {
+count++;
+}
- /*
- * Once stripped, the corrupted pointer is of the form 0x...wxyz.
- * We expect the autia to indicate failure, producing a pointer of the
- * form 0x000ewxyz. Use xpaci and != for the test, rather than
- * extracting explicit bits from the top, because the location of the
- * error code "e" depends on the configuration of virtual memory.
- */
- assert(x != y);
- return 0;
+}
+perc = (float) count / (float) TESTS;
+printf("Checks Passed: %0.2f%%", perc * 100.0);
+assert(perc > 0.95);
+return 0;
}
--
2.20.1
[PATCH v2 14/19] target/riscv: progressively load the instruction during decode
The plugin system would throw up a harmless warning when it detected
that a disassembly of an instruction didn't use all it's bytes. Fix
the riscv decoder to only load the instruction bytes it needs as it
needs them.
This drops opcode from the ctx in favour if passing the appropriately
sized opcode down a few levels of the decode.
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
---
v2
- use extract16 for uint16_t opcodes
squash! target/riscv: progressively load the instruction during decode
---
target/riscv/instmap.h | 8
target/riscv/translate.c | 40 +---
2 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/target/riscv/instmap.h b/target/riscv/instmap.h
index f8ad7d60fd5..40b6d2b64de 100644
--- a/target/riscv/instmap.h
+++ b/target/riscv/instmap.h
@@ -344,8 +344,8 @@ enum {
#define GET_C_LW_IMM(inst) ((extract32(inst, 6, 1) << 2) \
| (extract32(inst, 10, 3) << 3) \
| (extract32(inst, 5, 1) << 6))
-#define GET_C_LD_IMM(inst) ((extract32(inst, 10, 3) << 3) \
-| (extract32(inst, 5, 2) << 6))
+#define GET_C_LD_IMM(inst) ((extract16(inst, 10, 3) << 3) \
+| (extract16(inst, 5, 2) << 6))
#define GET_C_J_IMM(inst) ((extract32(inst, 3, 3) << 1) \
| (extract32(inst, 11, 1) << 4) \
| (extract32(inst, 2, 1) << 5) \
@@ -363,7 +363,7 @@ enum {
#define GET_C_RD(inst) GET_RD(inst)
#define GET_C_RS1(inst) GET_RD(inst)
#define GET_C_RS2(inst) extract32(inst, 2, 5)
-#define GET_C_RS1S(inst)(8 + extract32(inst, 7, 3))
-#define GET_C_RS2S(inst)(8 + extract32(inst, 2, 3))
+#define GET_C_RS1S(inst)(8 + extract16(inst, 7, 3))
+#define GET_C_RS2S(inst)(8 + extract16(inst, 2, 3))
#endif
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 14dc71156be..d5de7f468a7 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -44,7 +44,6 @@ typedef struct DisasContext {
/* pc_succ_insn points to the instruction following base.pc_next */
target_ulong pc_succ_insn;
target_ulong priv_ver;
-uint32_t opcode;
uint32_t mstatus_fs;
uint32_t misa;
uint32_t mem_idx;
@@ -492,45 +491,45 @@ static void gen_set_rm(DisasContext *ctx, int rm)
tcg_temp_free_i32(t0);
}
-static void decode_RV32_64C0(DisasContext *ctx)
+static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
{
-uint8_t funct3 = extract32(ctx->opcode, 13, 3);
-uint8_t rd_rs2 = GET_C_RS2S(ctx->opcode);
-uint8_t rs1s = GET_C_RS1S(ctx->opcode);
+uint8_t funct3 = extract16(opcode, 13, 3);
+uint8_t rd_rs2 = GET_C_RS2S(opcode);
+uint8_t rs1s = GET_C_RS1S(opcode);
switch (funct3) {
case 3:
#if defined(TARGET_RISCV64)
/* C.LD(RV64/128) -> ld rd', offset[7:3](rs1')*/
gen_load_c(ctx, OPC_RISC_LD, rd_rs2, rs1s,
- GET_C_LD_IMM(ctx->opcode));
+ GET_C_LD_IMM(opcode));
#else
/* C.FLW (RV32) -> flw rd', offset[6:2](rs1')*/
gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
-GET_C_LW_IMM(ctx->opcode));
+GET_C_LW_IMM(opcode));
#endif
break;
case 7:
#if defined(TARGET_RISCV64)
/* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
gen_store_c(ctx, OPC_RISC_SD, rs1s, rd_rs2,
- GET_C_LD_IMM(ctx->opcode));
+ GET_C_LD_IMM(opcode));
#else
/* C.FSW (RV32) -> fsw rs2', offset[6:2](rs1')*/
gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
- GET_C_LW_IMM(ctx->opcode));
+ GET_C_LW_IMM(opcode));
#endif
break;
}
}
-static void decode_RV32_64C(DisasContext *ctx)
+static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
{
-uint8_t op = extract32(ctx->opcode, 0, 2);
+uint8_t op = extract16(opcode, 0, 2);
switch (op) {
case 0:
-decode_RV32_64C0(ctx);
+decode_RV32_64C0(ctx, opcode);
break;
}
}
@@ -709,22 +708,25 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
/* Include the auto-generated decoder for 16 bit insn */
#include "decode_insn16.inc.c"
-static void decode_opc(DisasContext *ctx)
+static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
{
/* check for compressed insn */
-if (extract32(ctx->opcode, 0, 2) != 3) {
+if (extract16(opcode, 0, 2) != 3) {
if (!has_ext(ctx, RVC)) {
gen_exception_illegal(ctx);
} else {
ctx->pc_succ_insn = ctx->base.pc_next + 2;
-if (!decode_insn16(ctx, ctx->opcode)) {
+if (!decode_insn16(ctx, opcode)) {
/* fall back to old decoder
Re: [PATCH v2 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later
On Thu, Feb 13, 2020 at 03:34:25PM +0100, Greg Kurz wrote:
> On Thu, 13 Feb 2020 11:58:36 +1100
> David Gibson wrote:
>
> > PAPR specifies a kind of odd, paravirtualized PCI bus, which looks to
> > the guess mostly like classic PCI, even if some of the individual
> > devices on the bus are PCI Express. One consequence of that is that
> > virtio-pci devices still default to being in transitional mode, though
> > legacy mode is now disabled by default on current q35 x86 machine
> > types.
> >
> > Legacy mode virtio devices aren't really necessary any more, and are
> > causing some problems for future changes. Therefore, for the
> > pseries-5.0 machine type (and onwards), switch to modern-only
> > virtio-pci devices by default.
> >
> > This does mean we no longer support guest kernels prior to 4.0, unless
> > they have modern virtio support backported (which some distro kernels
> > like that in RHEL7 do).
> >
> > Signed-off-by: David Gibson
> > ---
> > hw/ppc/spapr.c | 14 +-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index cb220fde45..6e1e467cc6 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -65,6 +65,7 @@
> >
> > #include "hw/pci/pci.h"
> > #include "hw/scsi/scsi.h"
> > +#include "hw/virtio/virtio-pci.h"
> > #include "hw/virtio/virtio-scsi.h"
> > #include "hw/virtio/vhost-scsi-common.h"
> >
> > @@ -4567,7 +4568,14 @@ static void
> > spapr_machine_latest_class_options(MachineClass *mc)
> > */
> > static void spapr_machine_5_0_class_options(MachineClass *mc)
> > {
> > -/* Defaults for the latest behaviour inherited from the base class */
>
> Hmm... and so it seems we still have to carry this around when we
> add a new machine version. At least, that's what I had to do when
> adding a dummy 5.1 machine. This is because it is the old machine
> type that calls the class_options() function of the new one, not
> the other way around.
Uh.. no. It can just go in latest_class_options. I thought I'd
already moved it there, but obviously not. I've fixed that up for the
next spin.
> I was thinking about adapting Michael's patch. Instead of having
> a class_options() function that we only call for the latest
> machine type, we need a function that sets the default behaviour
> and call it for all machine types (which can still change the
> behaviour in their own class_options() function).
This will be confusing in a different way. It will reset the default
options on each of the chained old machine types, which means anything
set in the "default" options needs to be overriden in *all* old
machine types that don't want it, not just the latest which doesn't
want it.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[PATCH v2 16/19] tests/tcg: give debug builds a little bit longer
When combined with heavy plugins we occasionally hit the timeouts. Signed-off-by: Alex Bennée --- tests/tcg/Makefile.target | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target index 3c7421a356e..b3cff3cad1a 100644 --- a/tests/tcg/Makefile.target +++ b/tests/tcg/Makefile.target @@ -79,7 +79,7 @@ QEMU_OPTS= # If TCG debugging is enabled things are a lot slower ifeq ($(CONFIG_DEBUG_TCG),y) -TIMEOUT=45 +TIMEOUT=60 else TIMEOUT=15 endif @@ -137,7 +137,7 @@ PLUGINS=$(notdir $(wildcard $(PLUGIN_DIR)/*.so)) $(foreach p,$(PLUGINS), \ $(foreach t,$(TESTS),\ $(eval run-plugin-$(t)-with-$(p): $t $p) \ - $(eval run-plugin-$(t)-with-$(p): TIMEOUT=30) \ + $(eval run-plugin-$(t)-with-$(p): TIMEOUT=60) \ $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p endif -- 2.20.1
Re: [PATCH v5 1/6] hw/hppa/dino.c: Improve emulation of Dino PCI chip
On 2/13/20 12:37 AM, Philippe Mathieu-Daudé wrote:
> Hi Sven, Helge.
>
> On 12/20/19 10:15 PM, Sven Schnelle wrote:
>> From: Helge Deller
>>
>> The tests of the dino chip with the Online-diagnostics CD
>> ("ODE DINOTEST") now succeeds.
>> Additionally add some qemu trace events.
>>
>> Signed-off-by: Helge Deller
>> Signed-off-by: Sven Schnelle
>> Reviewed-by: Philippe Mathieu-Daudé
>> ---
>> MAINTAINERS | 2 +-
>> hw/hppa/dino.c | 97 +---
>> hw/hppa/trace-events | 5 +++
>> 3 files changed, 89 insertions(+), 15 deletions(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 387879aebc..e333bc67a4 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -876,7 +876,7 @@ F: hw/*/etraxfs_*.c
>> HP-PARISC Machines
>> --
>> -Dino
>> +HP B160L
>> M: Richard Henderson
>> R: Helge Deller
>> S: Odd Fixes
>> diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
>> index ab6969b45f..9797a7f0d9 100644
>> --- a/hw/hppa/dino.c
>> +++ b/hw/hppa/dino.c
>> @@ -1,7 +1,7 @@
>> /*
>> - * HP-PARISC Dino PCI chipset emulation.
>> + * HP-PARISC Dino PCI chipset emulation, as in B160L and similiar
>> machines
>> *
>> - * (C) 2017 by Helge Deller
>> + * (C) 2017-2019 by Helge Deller
>> *
>> * This work is licensed under the GNU GPL license version 2 or later.
>> *
>> @@ -21,6 +21,7 @@
>> #include "migration/vmstate.h"
>> #include "hppa_sys.h"
>> #include "exec/address-spaces.h"
>> +#include "trace.h"
>> #define TYPE_DINO_PCI_HOST_BRIDGE "dino-pcihost"
>> @@ -82,11 +83,28 @@
>> #define DINO_PCI_HOST_BRIDGE(obj) \
>> OBJECT_CHECK(DinoState, (obj), TYPE_DINO_PCI_HOST_BRIDGE)
>> +#define DINO800_REGS ((DINO_TLTIM - DINO_GMASK) / 4)
>
> Coverity noticed (CID 1419394) this should be '(1 + (DINO_TLTIM -
> DINO_GMASK) / 4)' (13 registers).
>
>> +static const uint32_t reg800_keep_bits[DINO800_REGS] = {
>> + MAKE_64BIT_MASK(0, 1),
>> + MAKE_64BIT_MASK(0, 7),
>> + MAKE_64BIT_MASK(0, 7),
>> + MAKE_64BIT_MASK(0, 8),
>> + MAKE_64BIT_MASK(0, 7),
>> + MAKE_64BIT_MASK(0, 9),
>> + MAKE_64BIT_MASK(0, 32),
>
> Looking at the datasheet pp. 30, this register is 'Undefined'.
> We should report GUEST_ERROR if it is accessed.
>
>> + MAKE_64BIT_MASK(0, 8),
>> + MAKE_64BIT_MASK(0, 30),
>> + MAKE_64BIT_MASK(0, 25),
>
> Still looking at the datasheet (pp. 37) PCIROR is 24-bit (not 25).
>
>> + MAKE_64BIT_MASK(0, 22),
>
> Here you are missing register 0x82c...
>
>> + MAKE_64BIT_MASK(0, 9),
>> +};
>> +
>
> Altogether:
>
> static const uint32_t reg800_keep_bits[DINO800_REGS] = {
> MAKE_64BIT_MASK(0, 1), /* GMASK */
> MAKE_64BIT_MASK(0, 7), /* PAMR */
> MAKE_64BIT_MASK(0, 7), /* PAPR */
> MAKE_64BIT_MASK(0, 8), /* DAMODE */
> MAKE_64BIT_MASK(0, 7), /* PCICMD */
> MAKE_64BIT_MASK(0, 9), /* PCISTS */
> MAKE_64BIT_MASK(0, 0), /* Undefined */
> MAKE_64BIT_MASK(0, 8), /* MLTIM */
> MAKE_64BIT_MASK(0, 30), /* BRDG_FEAT */
> MAKE_64BIT_MASK(0, 24), /* PCIROR */
> MAKE_64BIT_MASK(0, 22), /* PCIWOR */
> MAKE_64BIT_MASK(0, 0), /* Undocumented */
> MAKE_64BIT_MASK(0, 9), /* TLTIM */
> };
>
>> typedef struct DinoState {
>> PCIHostState parent_obj;
>> /* PCI_CONFIG_ADDR is parent_obj.config_reg, via
>> pci_host_conf_be_ops,
>> so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. */
>> + uint32_t config_reg_dino; /* keep original copy, including 2
>> lowest bits */
>> uint32_t iar0;
>> uint32_t iar1;
>> @@ -94,8 +112,12 @@ typedef struct DinoState {
>> uint32_t ipr;
>> uint32_t icr;
>> uint32_t ilr;
>> + uint32_t io_fbb_en;
>> uint32_t io_addr_en;
>> uint32_t io_control;
>> + uint32_t toc_addr;
>> +
>> + uint32_t reg800[DINO800_REGS];
>> MemoryRegion this_mem;
>> MemoryRegion pci_mem;
>> @@ -106,8 +128,6 @@ typedef struct DinoState {
>> MemoryRegion bm_ram_alias;
>> MemoryRegion bm_pci_alias;
>> MemoryRegion bm_cpu_alias;
>> -
>> - MemoryRegion cpu0_eir_mem;
>> } DinoState;
>> /*
>> @@ -122,6 +142,8 @@ static void gsc_to_pci_forwarding(DinoState *s)
>> tmp = extract32(s->io_control, 7, 2);
>> enabled = (tmp == 0x01);
>> io_addr_en = s->io_addr_en;
>> + /* Mask out first (=firmware) and last (=Dino) areas. */
>> + io_addr_en &= ~(BIT(31) | BIT(0));
>> memory_region_transaction_begin();
>> for (i = 1; i < 31; i++) {
>> @@ -142,6 +164,8 @@ static bool dino_chip_mem_valid(void *opaque,
>> hwaddr addr,
>> unsigned size, bool is_write,
>> MemTxAttrs attrs)
>> {
>> + bool ret = false;
>> +
>> switch (addr) {
>> case DINO_IAR0:
>> case DINO_IAR1:
>> @@ -152,16 +176,22 @@ static bool
Re: [PATCH v2 07/30] qapi/block-core.json: Use literal block for ascii art
6:59 PM Čet, 13.02.2020. Peter Maydell је написао/ла: > > The ascii-art graph Just out of couriousity, are unicode characters allowed in rst files? The boxes could've been rendered in a much more beautifull way using "lines and corners" group of unicode characters. Aleksandar > in the BlockLatencyHistogramInfo documentation > doesn't render correctly, because the whitespace is collapsed. > > Use the '|' format that emits a literal 'example' block so the graph > is displayed correctly. > > Strictly the texinfo generated is still wrong because each line > goes into its own @example environment, but it renders better > than what we had before. > > Fixing this rendering is a necessary prerequisite for the rST > generator, which otherwise complains about the inconsistent > indentation in the ascii-art graph. > > Signed-off-by: Peter Maydell > --- > v1->v2: tweaked commit message, made graph still line up > with preceding paragraph text > --- > qapi/block-core.json | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index ef94a296868..db9ca688d49 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -550,13 +550,13 @@ > #For the example above, @bins may be something like [3, 1, 5, 2], > #and corresponding histogram looks like: > # > -#5| * > -#4| * > -#3| * * > -#2| * ** > -#1| **** > -# +-- > -# 10 50 100 > +# | 5| * > +# | 4| * > +# | 3| * * > +# | 2| * ** > +# | 1| **** > +# | +-- > +# | 10 50 100 > # > # Since: 4.0 > ## > -- > 2.20.1 > >
[PATCH v2 17/19] tcg: save vaddr temp for plugin usage
From: Richard Henderson
While do_gen_mem_cb does copy (via extu_tl_i64) vaddr into a new temp
this won't help if the vaddr temp gets clobbered by the actual
load/store op. To avoid this clobbering we explicitly copy vaddr
before the op to ensure it is live my the time we do the
instrumentation.
Suggested-by: Richard Henderson
Signed-off-by: Alex Bennée
Cc: qemu-sta...@nongnu.org
---
tcg/tcg-op.c | 23 ---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 7d782002e3f..e2e25ebf7db 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -2794,13 +2794,26 @@ static void tcg_gen_req_mo(TCGBar type)
}
}
+static inline TCGv plugin_prep_mem_callbacks(TCGv vaddr)
+{
+#ifdef CONFIG_PLUGIN
+if (tcg_ctx->plugin_insn != NULL) {
+/* Save a copy of the vaddr for use after a load. */
+TCGv temp = tcg_temp_new();
+tcg_gen_mov_tl(temp, vaddr);
+return temp;
+}
+#endif
+return vaddr;
+}
+
static inline void plugin_gen_mem_callbacks(TCGv vaddr, uint16_t info)
{
#ifdef CONFIG_PLUGIN
-if (tcg_ctx->plugin_insn == NULL) {
-return;
+if (tcg_ctx->plugin_insn != NULL) {
+plugin_gen_empty_mem_callback(vaddr, info);
+tcg_temp_free(vaddr);
}
-plugin_gen_empty_mem_callback(vaddr, info);
#endif
}
@@ -2822,6 +2835,7 @@ void tcg_gen_qemu_ld_i32(TCGv_i32 val, TCGv addr, TCGArg
idx, MemOp memop)
}
}
+addr = plugin_prep_mem_callbacks(addr);
gen_ldst_i32(INDEX_op_qemu_ld_i32, val, addr, memop, idx);
plugin_gen_mem_callbacks(addr, info);
@@ -2868,6 +2882,7 @@ void tcg_gen_qemu_st_i32(TCGv_i32 val, TCGv addr, TCGArg
idx, MemOp memop)
memop &= ~MO_BSWAP;
}
+addr = plugin_prep_mem_callbacks(addr);
gen_ldst_i32(INDEX_op_qemu_st_i32, val, addr, memop, idx);
plugin_gen_mem_callbacks(addr, info);
@@ -2905,6 +2920,7 @@ void tcg_gen_qemu_ld_i64(TCGv_i64 val, TCGv addr, TCGArg
idx, MemOp memop)
}
}
+addr = plugin_prep_mem_callbacks(addr);
gen_ldst_i64(INDEX_op_qemu_ld_i64, val, addr, memop, idx);
plugin_gen_mem_callbacks(addr, info);
@@ -2967,6 +2983,7 @@ void tcg_gen_qemu_st_i64(TCGv_i64 val, TCGv addr, TCGArg
idx, MemOp memop)
memop &= ~MO_BSWAP;
}
+addr = plugin_prep_mem_callbacks(addr);
gen_ldst_i64(INDEX_op_qemu_st_i64, val, addr, memop, idx);
plugin_gen_mem_callbacks(addr, info);
--
2.20.1
[PATCH v2 12/19] tests/plugin: prevent uninitialized warning
From: Chen Qun
According to the glibc function requirements, we need initialise
the variable. Otherwise there will be compilation warnings:
glib-autocleanups.h:28:3: warning: ‘out’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
g_free (*pp);
^~~~
Reported-by: Euler Robot
Signed-off-by: Chen Qun
Reviewed-by: Thomas Huth
Message-Id: <20200206093238.203984-1-kuhn.chen...@huawei.com>
[AJB: uses Thomas's single line allocation]
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
---
tests/plugin/bb.c | 6 +++---
tests/plugin/insn.c | 3 +--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index f30bea08dcc..df19fd359df 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -22,9 +22,9 @@ static bool do_inline;
static void plugin_exit(qemu_plugin_id_t id, void *p)
{
-g_autofree gchar *out;
-out = g_strdup_printf("bb's: %" PRIu64", insns: %" PRIu64 "\n",
- bb_count, insn_count);
+g_autofree gchar *out = g_strdup_printf(
+"bb's: %" PRIu64", insns: %" PRIu64 "\n",
+bb_count, insn_count);
qemu_plugin_outs(out);
}
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index 0a8f5ae..a9a6e412373 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -44,8 +44,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
static void plugin_exit(qemu_plugin_id_t id, void *p)
{
-g_autofree gchar *out;
-out = g_strdup_printf("insns: %" PRIu64 "\n", insn_count);
+g_autofree gchar *out = g_strdup_printf("insns: %" PRIu64 "\n",
insn_count);
qemu_plugin_outs(out);
}
--
2.20.1
[PATCH v2 15/19] tests/plugins: make howvec clean-up after itself.
TCG plugins are responsible for their own memory usage and although
the plugin_exit is tied to the end of execution in this case it is
still poor practice. Ensure we delete the hash table and related data
when we are done to be a good plugin citizen.
Signed-off-by: Alex Bennée
Reviewed-by: Robert Foley
Reviewed-by: Richard Henderson
---
v2
- re-use counts for g_list_sort() as it modifies list
- drop it list
squash! tests/plugins: make howvec clean-up after itself.
---
tests/plugin/howvec.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/tests/plugin/howvec.c b/tests/plugin/howvec.c
index 4ca555e1239..3b9a6939f23 100644
--- a/tests/plugin/howvec.c
+++ b/tests/plugin/howvec.c
@@ -163,6 +163,13 @@ static gint cmp_exec_count(gconstpointer a, gconstpointer
b)
return ea->count > eb->count ? -1 : 1;
}
+static void free_record(gpointer data)
+{
+InsnExecCount *rec = (InsnExecCount *) data;
+g_free(rec->insn);
+g_free(rec);
+}
+
static void plugin_exit(qemu_plugin_id_t id, void *p)
{
g_autoptr(GString) report = g_string_new("Instruction Classes:\n");
@@ -195,30 +202,31 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
counts = g_hash_table_get_values(insns);
if (counts && g_list_next(counts)) {
-GList *it;
-
g_string_append_printf(report,"Individual Instructions:\n");
+counts = g_list_sort(counts, cmp_exec_count);
-it = g_list_sort(counts, cmp_exec_count);
-
-for (i = 0; i < limit && it->next; i++, it = it->next) {
-InsnExecCount *rec = (InsnExecCount *) it->data;
-g_string_append_printf(report, "Instr: %-24s\t(%ld
hits)\t(op=%#08x/%s)\n",
+for (i = 0; i < limit && g_list_next(counts);
+ i++, counts = g_list_next(counts)) {
+InsnExecCount *rec = (InsnExecCount *) counts->data;
+g_string_append_printf(report,
+ "Instr: %-24s\t(%ld hits)\t(op=%#08x/%s)\n",
rec->insn,
rec->count,
rec->opcode,
rec->class ?
rec->class->class : "un-categorised");
}
-g_list_free(it);
+g_list_free(counts);
}
+g_hash_table_destroy(insns);
+
qemu_plugin_outs(report->str);
}
static void plugin_init(void)
{
-insns = g_hash_table_new(NULL, g_direct_equal);
+insns = g_hash_table_new_full(NULL, g_direct_equal, NULL, _record);
}
static void vcpu_insn_exec_before(unsigned int cpu_index, void *udata)
--
2.20.1
Re: [PATCH v2 0/2] spapr: Use vIOMMU translation for virtio by default
On Thu, Feb 13, 2020 at 12:46:43PM +0100, Greg Kurz wrote: > On Thu, 13 Feb 2020 11:58:35 +1100 > David Gibson wrote: > > > Upcoming Secure VM support for pSeries machines introduces some > > complications for virtio, since the transfer buffers need to be > > explicitly shared so that the hypervisor can access them. > > > > While it's not strictly speaking dependent on it, the fact that virtio > > devices bypass normal platform IOMMU translation complicates the issue > > on the guest side. Since there are some significan downsides to > > bypassing the vIOMMU anyway, let's just disable that. > > > > There's already a flag to do this in virtio, just turn it on by > > default for forthcoming pseries machine types. > > > > Any opinions on whether dropping support for the older guest kernels > > is acceptable at this point? > > > > As expected, this breaks compatibility with existing RHEL 6.10 guests. Each > patch in this series requires an extra -global option to be specified on > the command line in order to boot successfully. > > Patch 1: -global virtio-pci.disable-legacy=auto > Patch 2: -global virtio-pci.iommu_platform=off Right, or setting an older machine type. > As seen on the RH site [1], RHEL6 will reach "End of Maintenance Support > or Maintenance Support 2 (Product retirement)" on November 30, 2020 and > "End of Extended Life-cycle Support" on June 30, 2024. > > Not sure if it's okay to drop support for RHEL6 this soon. Hm, yeah. I'm happy enough to do this upstream, downstream will require some discussion. > RHEL 7.7 guests seem to be unaffected. Yeah, I already checked and RHEL7 has backported support for modern virtio and the iommu platform flag. > > [1] https://access.redhat.com/support/policy/updates/errata/#Life_Cycle_Dates > > > Changes since v1: > > * Added information on which guest kernel versions will no longer > >work with these changes > > * Use Michael Tsirkin's suggested better way of handling the machine > >type change > > > > David Gibson (2): > > spapr: Disable legacy virtio devices for pseries-5.0 and later > > spapr: Enable virtio iommu_platform=on by default > > > > hw/ppc/spapr.c | 16 +++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[PATCH v2 18/19] tests/tcg: fix typo in configure.sh test for v8.3
Although most people use the docker images this can trip up on developer systems with actual valid cross-compilers! Signed-off-by: Alex Bennée --- tests/tcg/configure.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh index 9eb6ba3b7ea..eaaaff6233a 100755 --- a/tests/tcg/configure.sh +++ b/tests/tcg/configure.sh @@ -228,7 +228,7 @@ for target in $target_list; do echo "CROSS_CC_HAS_SVE=y" >> $config_target_mak fi if do_compiler "$target_compiler" $target_compiler_cflags \ - -march=-march=armv8.3-a -o $TMPE $TMPC; then + -march=armv8.3-a -o $TMPE $TMPC; then echo "CROSS_CC_HAS_ARMV8_3=y" >> $config_target_mak fi ;; -- 2.20.1
[PATCH v2 09/19] tracing: only allow -trace to override -D if set
Otherwise any -D settings the user may have made get ignored.
Signed-off-by: Alex Bennée
---
trace/control.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/trace/control.c b/trace/control.c
index 6c775e68eba..2ffe0008184 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -226,10 +226,15 @@ void trace_init_file(const char *file)
#ifdef CONFIG_TRACE_SIMPLE
st_set_trace_file(file);
#elif defined CONFIG_TRACE_LOG
-/* If both the simple and the log backends are enabled, "--trace file"
- * only applies to the simple backend; use "-D" for the log backend.
+/*
+ * If both the simple and the log backends are enabled, "--trace file"
+ * only applies to the simple backend; use "-D" for the log
+ * backend. However we should only override -D if we actually have
+ * something to override it with.
*/
-qemu_set_log_filename(file, _fatal);
+if (file) {
+qemu_set_log_filename(file, _fatal);
+}
#else
if (file) {
fprintf(stderr, "error: --trace file=...: "
--
2.20.1
Re: [PATCH] spapr: Rework hash<->radix transitions at CAS
On Thu, Feb 13, 2020 at 04:38:38PM +0100, Greg Kurz wrote:
> Until the CAS negotiation is over, an HPT can be allocated on three
> different paths:
>
> 1) during machine reset if the host doesn't support radix,
>
> 2) during CAS if the guest wants hash and doesn't support HPT resizing,
>in which case we pre-emptively resize the HPT to accomodate maxram,
>
> 3) during CAS if no CAS reboot was requested, the guest wants hash but
>we're currently configured for radix.
>
> Depending on the various combinations of host or guest MMU support,
> HPT resizing guest support and the possibility of a CAS reboot, it
> is quite hard to know which of these allocates the HPT that will
> be ultimately used by the guest that wants to do hash. Also, some of
> them have bugs:
>
> - 2) calls spapr_reallocate_hpt() instead of spapr_setup_hpt_and_vrma()
> and thus doesn't update the VRMA size, even though we've just extended
> the HPT. Not sure what issues this can cause,
>
> - 3) doesn't check for HPT resizing support and will always allocate a
> small HPT based on the initial RAM size. This caps the total amount of
> RAM the guest can see, especially if maxram is much higher than the
> initial ram.
>
> We only support guests that do CAS and we already assume that the HPT
> isn't being used when we do the pre-emptive resizing at CAS. It thus
> seems reasonable to only allocate the HPT at the end of CAS, when no
> CAS reboot was requested.
>
> Consolidate the logic so that we only create the HPT during 3), ie.
> when we're done with the CAS reboot cycles, and ensure HPT resizing
> is taken into account. This fixes the radix->hash transition for
> all cases.
Uh.. I'm pretty sure this can't work for KVM on a POWER8 host. We
need the HPT at all times there, or there's nowhere to put VRMA
entries, so we can't run even in real mode.
> The guest can theoretically call CAS several times, without a CAS
> reboot in between. Linux guests don't do that, but better safe than
> sorry, let's ensure we can also handle the symmetrical hash->radix
> transition correctly: free the HPT and set the GR bit in PATE.
> An helper is introduced for the latter since this is already what
> we do during machine reset when going for radix.
>
> As a bonus, this removes one user of spapr->cas_reboot, which we
> want to get rid of in the future.
>
> Signed-off-by: Greg Kurz
> ---
> hw/ppc/spapr.c | 25 +++-
> hw/ppc/spapr_hcall.c | 59
>
> include/hw/ppc/spapr.h |1 +
> 3 files changed, 44 insertions(+), 41 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 828e2cc1359a..88bc0e4e3ca1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1573,9 +1573,19 @@ void spapr_setup_hpt_and_vrma(SpaprMachineState *spapr)
> {
> int hpt_shift;
>
> +/*
> + * HPT resizing is a bit of a special case, because when enabled
> + * we assume an HPT guest will support it until it says it
> + * doesn't, instead of assuming it won't support it until it says
> + * it does. Strictly speaking that approach could break for
> + * guests which don't make a CAS call, but those are so old we
> + * don't care about them. Without that assumption we'd have to
> + * make at least a temporary allocation of an HPT sized for max
> + * memory, which could be impossibly difficult under KVM HV if
> + * maxram is large.
> + */
> if ((spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED)
> -|| (spapr->cas_reboot
> -&& !spapr_ovec_test(spapr->ov5_cas, OV5_HPT_RESIZE))) {
> +|| !spapr_ovec_test(spapr->ov5_cas, OV5_HPT_RESIZE)) {
> hpt_shift = spapr_hpt_shift_for_ramsize(MACHINE(spapr)->maxram_size);
> } else {
> uint64_t current_ram_size;
> @@ -1604,6 +1614,12 @@ static int spapr_reset_drcs(Object *child, void
> *opaque)
> return 0;
> }
>
> +void spapr_reset_patb_entry(SpaprMachineState *spapr)
> +{
> +spapr->patb_entry = PATE1_GR;
> +spapr_set_all_lpcrs(LPCR_HR | LPCR_UPRT, LPCR_HR | LPCR_UPRT);
> +}
> +
> static void spapr_machine_reset(MachineState *machine)
> {
> SpaprMachineState *spapr = SPAPR_MACHINE(machine);
> @@ -1624,10 +1640,7 @@ static void spapr_machine_reset(MachineState *machine)
> * without a HPT because KVM will start them in radix mode.
> * Set the GR bit in PATE so that we know there is no HPT.
> */
> -spapr->patb_entry = PATE1_GR;
> -spapr_set_all_lpcrs(LPCR_HR | LPCR_UPRT, LPCR_HR | LPCR_UPRT);
> -} else {
> -spapr_setup_hpt_and_vrma(spapr);
> +spapr_reset_patb_entry(spapr);
> }
>
> qemu_devices_reset();
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index b8bb66b5c0d4..57ddf0fa6d05 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1677,6 +1677,7 @@ static target_ulong
>
[PATCH v2 10/19] docs/devel: document query handle lifetimes
I forgot to document the lifetime of handles in the developer documentation. Do so now. Signed-off-by: Alex Bennée Reviewed-by: Robert Foley Reviewed-by: Robert Foley --- docs/devel/tcg-plugins.rst | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst index 718eef00f22..a05990906cc 100644 --- a/docs/devel/tcg-plugins.rst +++ b/docs/devel/tcg-plugins.rst @@ -51,8 +51,17 @@ about how QEMU's translation works to the plugins. While there are conceptions such as translation time and translation blocks the details are opaque to plugins. The plugin is able to query select details of instructions and system configuration only through the -exported *qemu_plugin* functions. The types used to describe -instructions and events are opaque to the plugins themselves. +exported *qemu_plugin* functions. + +Query Handle Lifetime +- + +Each callback provides an opaque anonymous information handle which +can usually be further queried to find out information about a +translation, instruction or operation. The handles themselves are only +valid during the lifetime of the callback so it is important that any +information that is needed is extracted during the callback and saved +by the plugin. Usage = -- 2.20.1
[PATCH v2 13/19] qemu/bitops.h: Add extract8 and extract16
From: Yoshinori Sato
Signed-off-by: Yoshinori Sato
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20190607091116.49044-10-ys...@users.sourceforge.jp>
Tested-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
Signed-off-by: Alex Bennée
Message-Id: <20200212130311.127515-3-ys...@users.sourceforge.jp>
---
include/qemu/bitops.h | 38 ++
1 file changed, 38 insertions(+)
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 02c1ce6a5d4..f55ce8b320b 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -301,6 +301,44 @@ static inline uint32_t extract32(uint32_t value, int
start, int length)
return (value >> start) & (~0U >> (32 - length));
}
+/**
+ * extract8:
+ * @value: the value to extract the bit field from
+ * @start: the lowest bit in the bit field (numbered from 0)
+ * @length: the length of the bit field
+ *
+ * Extract from the 8 bit input @value the bit field specified by the
+ * @start and @length parameters, and return it. The bit field must
+ * lie entirely within the 8 bit word. It is valid to request that
+ * all 8 bits are returned (ie @length 8 and @start 0).
+ *
+ * Returns: the value of the bit field extracted from the input value.
+ */
+static inline uint8_t extract8(uint8_t value, int start, int length)
+{
+assert(start >= 0 && length > 0 && length <= 8 - start);
+return extract32(value, start, length);
+}
+
+/**
+ * extract16:
+ * @value: the value to extract the bit field from
+ * @start: the lowest bit in the bit field (numbered from 0)
+ * @length: the length of the bit field
+ *
+ * Extract from the 16 bit input @value the bit field specified by the
+ * @start and @length parameters, and return it. The bit field must
+ * lie entirely within the 16 bit word. It is valid to request that
+ * all 16 bits are returned (ie @length 16 and @start 0).
+ *
+ * Returns: the value of the bit field extracted from the input value.
+ */
+static inline uint16_t extract16(uint16_t value, int start, int length)
+{
+assert(start >= 0 && length > 0 && length <= 16 - start);
+return extract32(value, start, length);
+}
+
/**
* extract64:
* @value: the value to extract the bit field from
--
2.20.1
[PATCH v2 05/19] travis.yml: Test the s390-ccw build, too
From: Thomas Huth
Since we can now use a s390x host on Travis, we can also build and
test the s390-ccw bios images there. For this we have to make sure
that roms/SLOF is checked out, too, and then move the generated *.img
files to the right location before running the tests.
Signed-off-by: Thomas Huth
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Acked-by: Cornelia Huck
Message-Id: <20200206202543.7085-1-th...@redhat.com>
---
.travis.yml | 10 ++
1 file changed, 10 insertions(+)
diff --git a/.travis.yml b/.travis.yml
index 58870559515..ea13e071795 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -509,6 +509,16 @@ matrix:
env:
- TEST_CMD="make check check-tcg V=1"
- CONFIG="--disable-containers
--target-list=${MAIN_SOFTMMU_TARGETS},s390x-linux-user"
+ script:
+- ( cd ${SRC_DIR} ; git submodule update --init roms/SLOF )
+- BUILD_RC=0 && make -j${JOBS} || BUILD_RC=$?
+- |
+ if [ "$BUILD_RC" -eq 0 ] ; then
+ mv pc-bios/s390-ccw/*.img pc-bios/ ;
+ ${TEST_CMD} ;
+ else
+ $(exit $BUILD_RC);
+ fi
# Release builds
# The make-release script expect a QEMU version, so our tag must start
with a 'v'.
--
2.20.1
[PATCH v2 08/19] tests/iotests: be a little more forgiving on the size test
At least on ZFS this was failing as 512 was less than or equal to 512.
I suspect the reason is additional compression done by ZFS and however
qemu-img gets the actual size.
Loosen the criteria to make sure after is not bigger than before and
also dump the values in the report.
Signed-off-by: Alex Bennée
---
tests/qemu-iotests/214 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index 3500e0c47a2..6d1324cd157 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -125,9 +125,9 @@ $QEMU_IO -c "write -P 0xcc $offset $data_size" "json:{\
sizeB=$($QEMU_IMG info --output=json "$TEST_IMG" |
sed -n '/"actual-size":/ s/[^0-9]//gp')
-if [ $sizeA -le $sizeB ]
+if [ $sizeA -lt $sizeB ]
then
-echo "Compression ERROR"
+echo "Compression ERROR ($sizeA vs $sizeB)"
fi
$QEMU_IMG check --output=json "$TEST_IMG" |
--
2.20.1
[PATCH v2 07/19] travis.yml: single-thread build-tcg stages
This still seems to be a problem for Travis.
Signed-off-by: Alex Bennée
---
.travis.yml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 0612998958b..f4020dcc6c8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -400,7 +400,7 @@ jobs:
- name: "GCC check-tcg (some-softmmu)"
env:
- CONFIG="--enable-debug-tcg
--target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-- TEST_BUILD_CMD="make -j${JOBS} build-tcg"
+- TEST_BUILD_CMD="make build-tcg"
- TEST_CMD="make check-tcg"
- CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
@@ -409,7 +409,7 @@ jobs:
- name: "GCC plugins check-tcg (some-softmmu)"
env:
- CONFIG="--enable-plugins --enable-debug-tcg
--target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-- TEST_BUILD_CMD="make -j${JOBS} build-tcg"
+- TEST_BUILD_CMD="make build-tcg"
- TEST_CMD="make check-tcg"
- CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
--
2.20.1
[PATCH v2 03/19] tests/rcutorture: better document locking of stats
This is pure code motion with no functional effect.
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
---
tests/rcutorture.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index e8b2169e7dd..256d24ed5ba 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -65,8 +65,6 @@
#include "qemu/rcu.h"
#include "qemu/thread.h"
-long long n_reads = 0LL;
-long n_updates = 0L;
int nthreadsrunning;
#define GOFLAG_INIT 0
@@ -78,11 +76,20 @@ static volatile int goflag = GOFLAG_INIT;
#define RCU_READ_RUN 1000
#define NR_THREADS 100
-static QemuMutex counts_mutex;
static QemuThread threads[NR_THREADS];
static struct rcu_reader_data *data[NR_THREADS];
static int n_threads;
+/*
+ * Statistical counts
+ *
+ * These are the sum of local counters at the end of a run.
+ * Updates are protected by a mutex.
+ */
+static QemuMutex counts_mutex;
+long long n_reads = 0LL;
+long n_updates = 0L;
+
static void create_thread(void *(*func)(void *))
{
if (n_threads >= NR_THREADS) {
@@ -230,8 +237,9 @@ struct rcu_stress {
struct rcu_stress rcu_stress_array[RCU_STRESS_PIPE_LEN] = { { 0 } };
struct rcu_stress *rcu_stress_current;
int rcu_stress_idx;
-
int n_mberror;
+
+/* Updates protected by counts_mutex */
long long rcu_stress_count[RCU_STRESS_PIPE_LEN + 1];
--
2.20.1
[PATCH v2 11/19] plugins/core: add missing break in cb_to_tcg_flags
From: "Emilio G. Cota"
Reported-by: Robert Henry
Signed-off-by: Emilio G. Cota
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
Message-Id: <20200105072940.32204-1-c...@braap.org>
Cc: qemu-sta...@nongnu.org
---
plugins/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/core.c b/plugins/core.c
index 9e1b9e7a915..ed863011baf 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -286,6 +286,7 @@ static inline uint32_t cb_to_tcg_flags(enum
qemu_plugin_cb_flags flags)
switch (flags) {
case QEMU_PLUGIN_CB_RW_REGS:
ret = 0;
+break;
case QEMU_PLUGIN_CB_R_REGS:
ret = TCG_CALL_NO_WG;
break;
--
2.20.1
[PATCH v2 04/19] tests/rcutorture: mild documenting refactor of update thread
This is mainly to help with reasoning what the test is trying to do.
We can move rcu_stress_idx to a local variable as there is only ever
one updater thread. I've also added an assert to catch the case where
we end up updating the current structure to itself which is the only
way I can see the mberror cases we are seeing on Travis.
We shall see if the rcutorture test failures go away now.
Signed-off-by: Alex Bennée
---
tests/rcutorture.c | 36 +++-
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index 256d24ed5ba..9559f13cd1f 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -236,7 +236,6 @@ struct rcu_stress {
struct rcu_stress rcu_stress_array[RCU_STRESS_PIPE_LEN] = { { 0 } };
struct rcu_stress *rcu_stress_current;
-int rcu_stress_idx;
int n_mberror;
/* Updates protected by counts_mutex */
@@ -288,29 +287,48 @@ static void *rcu_read_stress_test(void *arg)
return NULL;
}
+/*
+ * Stress Test Updater
+ *
+ * The updater cycles around updating rcu_stress_current to point at
+ * one of the rcu_stress_array_entries and resets it's pipe_count. It
+ * then increments pipe_count of all the other entries. The pipe_count
+ * will be read under an rcu_read_lock() and distribution of values
+ * calculated. The final result gives an indication of how many
+ * previously current rcu_stress entries are in flight until the RCU
+ * cycle complete.
+ */
static void *rcu_update_stress_test(void *arg)
{
-int i;
-struct rcu_stress *p;
+int i, rcu_stress_idx = 0;
+struct rcu_stress *cp = atomic_read(_stress_current);
rcu_register_thread();
-
*(struct rcu_reader_data **)arg = _reader;
+
while (goflag == GOFLAG_INIT) {
g_usleep(1000);
}
+
while (goflag == GOFLAG_RUN) {
-i = rcu_stress_idx + 1;
-if (i >= RCU_STRESS_PIPE_LEN) {
-i = 0;
+struct rcu_stress *p;
+rcu_stress_idx++;
+if (rcu_stress_idx >= RCU_STRESS_PIPE_LEN) {
+rcu_stress_idx = 0;
}
-p = _stress_array[i];
+p = _stress_array[rcu_stress_idx];
+/* catching up with ourselves would be a bug */
+assert(p != cp);
p->mbtest = 0;
smp_mb();
p->pipe_count = 0;
p->mbtest = 1;
atomic_rcu_set(_stress_current, p);
-rcu_stress_idx = i;
+cp = p;
+/*
+ * New RCU structure is now live, update pipe counts on old
+ * ones.
+ */
for (i = 0; i < RCU_STRESS_PIPE_LEN; i++) {
if (i != rcu_stress_idx) {
rcu_stress_array[i].pipe_count++;
--
2.20.1
[PATCH v2 06/19] travis.yml: Fix Travis YAML configuration warnings
From: Wainer dos Santos Moschetta
This fixes the following warnings Travis has detected on the
YAML configuration:
- 'on root: missing os, using the default "linux"'
- 'on root: the key matrix is an alias for jobs, using jobs'
- 'on jobs.include.python: unexpected sequence, using the first value (3.5)'
- 'on jobs.include.python: unexpected sequence, using the first value (3.6)'
Signed-off-by: Wainer dos Santos Moschetta
Signed-off-by: Alex Bennée
Message-Id: <20200207210124.141119-2-waine...@redhat.com>
---
.travis.yml | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index ea13e071795..0612998958b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,7 @@
# The current Travis default is a VM based 16.04 Xenial on GCE
# Additional builds with specific requirements for a full VM need to
# be added as additional matrix: entries later on
+os: linux
dist: xenial
language: c
compiler:
@@ -113,7 +114,7 @@ after_script:
- if command -v ccache ; then ccache --show-stats ; fi
-matrix:
+jobs:
include:
- name: "GCC static (user)"
env:
@@ -297,8 +298,7 @@ matrix:
- CONFIG="--target-list=x86_64-softmmu"
- CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-default"
language: python
- python:
-- "3.5"
+ python: 3.5
- name: "GCC Python 3.6 (x86_64-softmmu)"
@@ -306,8 +306,7 @@ matrix:
- CONFIG="--target-list=x86_64-softmmu"
- CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-default"
language: python
- python:
-- "3.6"
+ python: 3.6
# Acceptance (Functional) tests
--
2.20.1
[PATCH v2 01/19] tests/tcg: include a skip runner for pauth3 with plugins
If we have plugins enabled we still need to have built the test to be able to run it. Signed-off-by: Alex Bennée --- tests/tcg/aarch64/Makefile.softmmu-target | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target index d2299b98b76..71f72cfbe34 100644 --- a/tests/tcg/aarch64/Makefile.softmmu-target +++ b/tests/tcg/aarch64/Makefile.softmmu-target @@ -70,4 +70,6 @@ pauth-3: $(call skip-test, "BUILD of $@", "missing compiler support") run-pauth-3: $(call skip-test, "RUN of pauth-3", "not built") +run-plugin-pauth-3-with-%: + $(call skip-test, "RUN of pauth-3 ($*)", "not built") endif -- 2.20.1
[PATCH v2 02/19] tests/rcutorture: update usage hint
Although documented in the comments we don't display all the various
invocations we can in the usage.
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
---
tests/rcutorture.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index 49311c82ea4..e8b2169e7dd 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -413,7 +413,8 @@ static void gtest_stress_10_5(void)
static void usage(int argc, char *argv[])
{
-fprintf(stderr, "Usage: %s [nreaders [ perf | stress ] ]\n", argv[0]);
+fprintf(stderr, "Usage: %s [nreaders [ [r|u]perf | stress [duration]]\n",
+argv[0]);
exit(-1);
}
--
2.20.1
[PATCH v2 00/19] testing and plugin updates
Hi, I've ended up combining my accumulated testing fixes with the plugin fixes as there is some cross-over between the two. On the testing side I still haven't seen rcutorture trip up on my branches but the final patch that light re-factors it needs to be reviewed. I've also added some fixes for pauth - both ensuring they compiler and tweaking pauth-4 to take into account the occasional authentication clashes. Plugin wise I have cleaned up the riscv parser to use extract16 where appropriate. We also managed to diagnose a bug in the address passing of the memory instrumentation which only showed up under alpha. The relevant patches have been Cc'ed to qemu-stable. The following patches need review: tests/tcg: take into account expected clashes pauth-4 tests/tcg: fix typo in configure.sh test for v8.3 tcg: save vaddr temp for plugin usage tests/tcg: give debug builds a little bit longer tracing: only allow -trace to override -D if set tests/iotests: be a little more forgiving on the size test travis.yml: single-thread build-tcg stages travis.yml: Fix Travis YAML configuration warnings tests/rcutorture: mild documenting refactor of update thread tests/tcg: include a skip runner for pauth3 with plugins Alex Bennée (13): tests/tcg: include a skip runner for pauth3 with plugins tests/rcutorture: update usage hint tests/rcutorture: better document locking of stats tests/rcutorture: mild documenting refactor of update thread travis.yml: single-thread build-tcg stages tests/iotests: be a little more forgiving on the size test tracing: only allow -trace to override -D if set docs/devel: document query handle lifetimes target/riscv: progressively load the instruction during decode tests/plugins: make howvec clean-up after itself. tests/tcg: give debug builds a little bit longer tests/tcg: fix typo in configure.sh test for v8.3 tests/tcg: take into account expected clashes pauth-4 Chen Qun (1): tests/plugin: prevent uninitialized warning Emilio G. Cota (1): plugins/core: add missing break in cb_to_tcg_flags Richard Henderson (1): tcg: save vaddr temp for plugin usage Thomas Huth (1): travis.yml: Test the s390-ccw build, too Wainer dos Santos Moschetta (1): travis.yml: Fix Travis YAML configuration warnings Yoshinori Sato (1): qemu/bitops.h: Add extract8 and extract16 docs/devel/tcg-plugins.rst| 13 +- include/qemu/bitops.h | 38 target/riscv/instmap.h| 8 ++-- plugins/core.c| 1 + target/riscv/translate.c | 40 + tcg/tcg-op.c | 23 -- tests/plugin/bb.c | 6 +-- tests/plugin/howvec.c | 26 +++ tests/plugin/insn.c | 3 +- tests/rcutorture.c| 55 +-- tests/tcg/aarch64/pauth-4.c | 54 +++--- trace/control.c | 11 +++-- .travis.yml | 23 +++--- tests/qemu-iotests/214| 4 +- tests/tcg/Makefile.target | 4 +- tests/tcg/aarch64/Makefile.softmmu-target | 2 + tests/tcg/configure.sh| 2 +- 17 files changed, 225 insertions(+), 88 deletions(-) -- 2.20.1
Re: [PATCH v2 2/2] tests/tcg/multiarch: Add tests for implemented alsa sound timer ioctls
3:26 PM Čet, 13.02.2020. Filip Bozuta је
написао/ла:
>
> +int main(int argc, char **argv)
> +{
> +char ioctls[15][35] = {"SNDRV_TIMER_IOCTL_PVERSION",
> + "SNDRV_TIMER_IOCTL_INFO",
> + "SNDRV_TIMER_IOCTL_NEXT_DEVICE",
> + "SNDRV_TIMER_IOCTL_PARAMS",
> + "SNDRV_TIMER_IOCTL_TREAD",
> + "SNDRV_TIMER_IOCTL_STATUS",
> + "SNDRV_TIMER_IOCTL_GINFO",
> + "SNDRV_TIMER_IOCTL_START",
> + "SNDRV_TIMER_IOCTL_GPARAMS",
> + "SNDRV_TIMER_IOCTL_STOP",
> + "SNDRV_TIMER_IOCTL_GSTATUS",
> + "SNDRV_TIMER_IOCTL_CONTINUE",
> + "SNDRV_TIMER_IOCTL_SELECT",
> + "SNDRV_TIMER_IOCTL_PAUSE"};
> +
> +bool (*const funcs[]) (int, bool) = {
> + test_pversion,
> + test_next_device,
> + test_tread,
> + test_ginfo,
> + test_gparams,
> + test_gstatus,
> + test_select,
> + test_info,
> + test_params,
> + test_status,
> + test_start,
> + test_pause,
> + test_continue,
> + test_stop,
> + NULL
> +};
> +
Order of these two arrays don't match, and that leads to the wrong choice
of test function later on in the code. For example, if one chooses
"SNDRV_TIMER_IOCTL_STATUS" in the command line, one will end up testing
"SNDRV_TIMER_IOCTL_GSTATUS", if one chooses "SNDRV_TIMER_IOCTL_INFO", one
will end up testing "SNDRV_TIMER_IOCTL_NEXT_DEVICE", etc. Nice feature
(ability to test just a single ioctl), but it needs to be fixed.
Thanks,
Aleksandar
Re: [PATCH] migration/postcopy: not necessary to discard all RAM at the beginning
On Thu, Feb 13, 2020 at 10:17:04AM +, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> ram_discard_range() unmap page for specific range. To be specific, this
>> clears related page table entries so that userfault would be triggered.
>> But this step is not necessary at the very beginning.
>>
>> ram_postcopy_incoming_init() is called when destination gets ADVISE
>> command. ADVISE command is sent when migration thread just starts, which
>> implies destination is not running yet. This means no page fault
>> happened and memory region's page tables entries are empty.
>>
>> This patch removes the discard at the beginning.
>>
>> Signed-off-by: Wei Yang
>> ---
>> migration/postcopy-ram.c | 46
>> migration/postcopy-ram.h | 7 --
>> migration/ram.c | 16 --
>> migration/ram.h | 1 -
>> migration/savevm.c | 4
>> 5 files changed, 74 deletions(-)
>>
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 5da6de8c8b..459be8e780 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -443,32 +443,6 @@ out:
>> return ret;
>> }
>>
>> -/*
>> - * Setup an area of RAM so that it *can* be used for postcopy later; this
>> - * must be done right at the start prior to pre-copy.
>> - * opaque should be the MIS.
>> - */
>> -static int init_range(RAMBlock *rb, void *opaque)
>> -{
>> -const char *block_name = qemu_ram_get_idstr(rb);
>> -void *host_addr = qemu_ram_get_host_addr(rb);
>> -ram_addr_t offset = qemu_ram_get_offset(rb);
>> -ram_addr_t length = qemu_ram_get_used_length(rb);
>> -trace_postcopy_init_range(block_name, host_addr, offset, length);
>> -
>> -/*
>> - * We need the whole of RAM to be truly empty for postcopy, so things
>> - * like ROMs and any data tables built during init must be zero'd
>> - * - we're going to get the copy from the source anyway.
>> - * (Precopy will just overwrite this data, so doesn't need the discard)
>> - */
>
>But this comment explains why we want to do the discard; we want to make
>sure that any memory that's been populated by the destination during the
>init process is discarded and replaced by content from the source.
>
OK, you are right. I missed the init stage.
>Dave
>
--
Wei Yang
Help you, Help me
[PATCH v6 5/8] multifd: Add zlib compression multifd support
Signed-off-by: Juan Quintela
Acked-by: Markus Armbruster
---
hw/core/qdev-properties.c| 2 +-
migration/Makefile.objs | 1 +
migration/multifd-zlib.c | 325 +++
migration/multifd.c | 6 +
migration/multifd.h | 4 +
qapi/migration.json | 3 +-
tests/qtest/migration-test.c | 6 +
7 files changed, 345 insertions(+), 2 deletions(-)
create mode 100644 migration/multifd-zlib.c
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index fa7edac020..db2a7abfb2 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -645,7 +645,7 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
const PropertyInfo qdev_prop_multifd_compression = {
.name = "MultiFDCompression",
.description = "multifd_compression values, "
- "none",
+ "none/zlib",
.enum_table = _lookup,
.get = get_enum,
.set = set_enum,
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index d3623d5f9b..0308caa5c5 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -8,6 +8,7 @@ common-obj-y += xbzrle.o postcopy-ram.o
common-obj-y += qjson.o
common-obj-y += block-dirty-bitmap.o
common-obj-y += multifd.o
+common-obj-y += multifd-zlib.o
common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
new file mode 100644
index 00..ab4ba75d75
--- /dev/null
+++ b/migration/multifd-zlib.c
@@ -0,0 +1,325 @@
+/*
+ * Multifd zlib compression implementation
+ *
+ * Copyright (c) 2020 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include
+#include "qemu/rcu.h"
+#include "exec/target_page.h"
+#include "qapi/error.h"
+#include "migration.h"
+#include "trace.h"
+#include "multifd.h"
+
+struct zlib_data {
+/* stream for compression */
+z_stream zs;
+/* compressed buffer */
+uint8_t *zbuff;
+/* size of compressed buffer */
+uint32_t zbuff_len;
+};
+
+/* Multifd zlib compression */
+
+/**
+ * zlib_send_setup: setup send side
+ *
+ * Setup each channel with zlib compression.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int zlib_send_setup(MultiFDSendParams *p, Error **errp)
+{
+uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+struct zlib_data *z = g_malloc0(sizeof(struct zlib_data));
+z_stream *zs = >zs;
+
+zs->zalloc = Z_NULL;
+zs->zfree = Z_NULL;
+zs->opaque = Z_NULL;
+if (deflateInit(zs, migrate_multifd_zlib_level()) != Z_OK) {
+g_free(z);
+error_setg(errp, "multifd %d: deflate init failed", p->id);
+return -1;
+}
+/* We will never have more than page_count pages */
+z->zbuff_len = page_count * qemu_target_page_size();
+z->zbuff_len *= 2;
+z->zbuff = g_try_malloc(z->zbuff_len);
+if (!z->zbuff) {
+deflateEnd(>zs);
+g_free(z);
+error_setg(errp, "multifd %d: out of memory for zbuff", p->id);
+return -1;
+}
+p->data = z;
+return 0;
+}
+
+/**
+ * zlib_send_cleanup: cleanup send side
+ *
+ * Close the channel and return memory.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void zlib_send_cleanup(MultiFDSendParams *p, Error **errp)
+{
+struct zlib_data *z = p->data;
+
+deflateEnd(>zs);
+g_free(z->zbuff);
+z->zbuff = NULL;
+g_free(p->data);
+p->data = NULL;
+}
+
+/**
+ * zlib_send_prepare: prepare date to be able to send
+ *
+ * Create a compressed buffer with all the pages that we are going to
+ * send.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ */
+static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+struct iovec *iov = p->pages->iov;
+struct zlib_data *z = p->data;
+z_stream *zs = >zs;
+uint32_t out_size = 0;
+int ret;
+uint32_t i;
+
+for (i = 0; i < used; i++) {
+uint32_t available = z->zbuff_len - out_size;
+int flush = Z_NO_FLUSH;
+
+if (i == used - 1) {
+flush = Z_SYNC_FLUSH;
+}
+
+zs->avail_in = iov[i].iov_len;
+zs->next_in = iov[i].iov_base;
+
+zs->avail_out = available;
+zs->next_out = z->zbuff + out_size;
+
+/*
+ * Welcome to deflate semantics
+ *
+ * We need to loop while:
+ * - return is Z_OK
+ * - there are stuff to be compressed
+ * - there are output space free
+ */
+do {
+ret = deflate(zs, flush);
+} while (ret == Z_OK && zs->avail_in && zs->avail_out);
+if (ret == Z_OK && zs->avail_in) {
+error_setg(errp,
[PATCH v6 6/8] configure: Enable test and libs for zstd
Add it to several build systems to make testing good. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- .gitlab-ci.yml| 1 + .travis.yml | 1 + configure | 30 +++ tests/docker/dockerfiles/centos7.docker | 3 +- .../dockerfiles/fedora-i386-cross.docker | 3 +- tests/docker/dockerfiles/fedora.docker| 3 +- tests/docker/dockerfiles/ubuntu.docker| 1 + tests/docker/dockerfiles/ubuntu1804.docker| 1 + tests/vm/fedora | 5 +++- tests/vm/freebsd | 3 ++ tests/vm/netbsd | 3 ++ tests/vm/openbsd | 3 ++ 12 files changed, 53 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c15e394f09..72f8b8aa51 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -21,6 +21,7 @@ build-system2: script: - apt-get install -y -qq libsdl2-dev libgcrypt-dev libbrlapi-dev libaio-dev libfdt-dev liblzo2-dev librdmacm-dev libibverbs-dev libibumad-dev + libzstd-dev - mkdir build - cd build - ../configure --enable-werror --target-list="tricore-softmmu unicore32-softmmu diff --git a/.travis.yml b/.travis.yml index 5887055951..dd17301f3b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -48,6 +48,7 @@ addons: - libusb-1.0-0-dev - libvdeplug-dev - libvte-2.91-dev + - libzstd-dev - sparse - uuid-dev - gcovr diff --git a/configure b/configure index 16f94cd96b..e6c12faef4 100755 --- a/configure +++ b/configure @@ -449,6 +449,7 @@ lzo="" snappy="" bzip2="" lzfse="" +zstd="" guest_agent="" guest_agent_with_vss="no" guest_agent_ntddscsi="no" @@ -1348,6 +1349,10 @@ for opt do ;; --disable-lzfse) lzfse="no" ;; + --disable-zstd) zstd="no" + ;; + --enable-zstd) zstd="yes" + ;; --enable-guest-agent) guest_agent="yes" ;; --disable-guest-agent) guest_agent="no" @@ -1801,6 +1806,8 @@ disabled with --disable-FEATURE, default is enabled if available: (for reading bzip2-compressed dmg images) lzfse support of lzfse compression library (for reading lzfse-compressed dmg images) + zstdsupport for zstd compression library + (for migration compression) seccomp seccomp support coroutine-pool coroutine freelist (better performance) glusterfs GlusterFS backend @@ -2415,6 +2422,24 @@ EOF fi fi +## +# zstd check + +if test "$zstd" != "no" ; then +if $pkg_config --exist libzstd ; then +zstd_cflags="$($pkg_config --cflags libzstd)" +zstd_libs="$($pkg_config --libs libzstd)" +LIBS="$zstd_libs $LIBS" +QEMU_CFLAGS="$QEMU_CFLAGS $zstd_cflags" +zstd="yes" +else +if test "$zstd" = "yes" ; then +feature_not_found "libzstd" "Install libzstd devel" +fi +zstd="no" +fi +fi + ## # libseccomp check @@ -6600,6 +6625,7 @@ echo "lzo support $lzo" echo "snappy support$snappy" echo "bzip2 support $bzip2" echo "lzfse support $lzfse" +echo "zstd support $zstd" echo "NUMA host support $numa" echo "libxml2 $libxml2" echo "tcmalloc support $tcmalloc" @@ -7173,6 +7199,10 @@ if test "$lzfse" = "yes" ; then echo "LZFSE_LIBS=-llzfse" >> $config_host_mak fi +if test "$zstd" = "yes" ; then + echo "CONFIG_ZSTD=y" >> $config_host_mak +fi + if test "$libiscsi" = "yes" ; then echo "CONFIG_LIBISCSI=m" >> $config_host_mak echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker index 562d65be9e..cdd72de7eb 100644 --- a/tests/docker/dockerfiles/centos7.docker +++ b/tests/docker/dockerfiles/centos7.docker @@ -33,6 +33,7 @@ ENV PACKAGES \ tar \ vte-devel \ xen-devel \ -zlib-devel +zlib-devel \ +libzstd-devel RUN yum install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker index 9106cf9ebe..cd16cd1bfa 100644 --- a/tests/docker/dockerfiles/fedora-i386-cross.docker +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker @@ -7,7 +7,8 @@ ENV PACKAGES \ gnutls-devel.i686 \ nettle-devel.i686 \ pixman-devel.i686 \ -zlib-devel.i686 +zlib-devel.i686 \ +libzstd-devel.i686 RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 987a3c170a..a658c0 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -92,7 +92,8 @@ ENV PACKAGES \
[PATCH v6 4/8] multifd: Add multifd-zlib-level parameter
This parameter specifies the zlib compression level. The next patch
will put it to use.
Signed-off-by: Juan Quintela
Acked-by: Markus Armbruster
Reviewed-by: Dr. David Alan Gilbert
---
migration/migration.c | 24
migration/migration.h | 1 +
monitor/hmp-cmds.c| 4
qapi/migration.json | 30 +++---
4 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index eb7b0a2df0..a09726f679 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -89,6 +89,8 @@
#define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
#define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
#define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
+/* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
+#define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
/* Background transfer rate for postcopy, 0 means unlimited, note
* that page requests can still exceed this limit.
@@ -801,6 +803,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
**errp)
params->multifd_channels = s->parameters.multifd_channels;
params->has_multifd_compression = true;
params->multifd_compression = s->parameters.multifd_compression;
+params->has_multifd_zlib_level = true;
+params->multifd_zlib_level = s->parameters.multifd_zlib_level;
params->has_xbzrle_cache_size = true;
params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
params->has_max_postcopy_bandwidth = true;
@@ -1208,6 +1212,13 @@ static bool migrate_params_check(MigrationParameters
*params, Error **errp)
return false;
}
+if (params->has_multifd_zlib_level &&
+(params->multifd_zlib_level > 9)) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zlib_level",
+ "is invalid, it should be in the range of 0 to 9");
+return false;
+}
+
if (params->has_xbzrle_cache_size &&
(params->xbzrle_cache_size < qemu_target_page_size() ||
!is_power_of_2(params->xbzrle_cache_size))) {
@@ -2254,6 +2265,15 @@ MultiFDCompression migrate_multifd_compression(void)
return s->parameters.multifd_compression;
}
+int migrate_multifd_zlib_level(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->parameters.multifd_zlib_level;
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
@@ -3544,6 +3564,9 @@ static Property migration_properties[] = {
DEFINE_PROP_MULTIFD_COMPRESSION("multifd-compression", MigrationState,
parameters.multifd_compression,
DEFAULT_MIGRATE_MULTIFD_COMPRESSION),
+DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
+ parameters.multifd_zlib_level,
+ DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
parameters.xbzrle_cache_size,
DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
@@ -3635,6 +3658,7 @@ static void migration_instance_init(Object *obj)
params->has_block_incremental = true;
params->has_multifd_channels = true;
params->has_multifd_compression = true;
+params->has_multifd_zlib_level = true;
params->has_xbzrle_cache_size = true;
params->has_max_postcopy_bandwidth = true;
params->has_max_cpu_throttle = true;
diff --git a/migration/migration.h b/migration/migration.h
index 59fea02482..c363ef9334 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -301,6 +301,7 @@ bool migrate_use_multifd(void);
bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
MultiFDCompression migrate_multifd_compression(void);
+int migrate_multifd_zlib_level(void);
int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 40525b5e95..8a39dff589 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1836,6 +1836,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict
*qdict)
}
p->multifd_compression = compress_type;
break;
+case MIGRATION_PARAMETER_MULTIFD_ZLIB_LEVEL:
+p->has_multifd_zlib_level = true;
+visit_type_int(v, param, >multifd_zlib_level, );
+break;
case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
p->has_xbzrle_cache_size = true;
visit_type_size(v, param, _size, );
diff --git a/qapi/migration.json b/qapi/migration.json
index 28593e2a2c..0845d926f2 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -602,6 +602,13 @@
# @multifd-compression: Which compression method to use.
# Defaults to none. (Since 5.0)
#
+# @multifd-zlib-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 9, where 0 means no compression, 1 means the best
+# compression speed, and 9 means
[PATCH v6 7/8] multifd: Add multifd-zstd-level parameter
This parameter specifies the zstd compression level. The next patch
will put it to use.
Signed-off-by: Juan Quintela
Acked-by: Markus Armbruster
---
migration/migration.c | 24
migration/migration.h | 1 +
monitor/hmp-cmds.c| 4
qapi/migration.json | 29 ++---
4 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index a09726f679..c1814a6861 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -91,6 +91,8 @@
#define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
/* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
#define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
+/* 0: means nocompress, 1: best speed, ... 20: best compress ratio */
+#define DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL 1
/* Background transfer rate for postcopy, 0 means unlimited, note
* that page requests can still exceed this limit.
@@ -805,6 +807,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
**errp)
params->multifd_compression = s->parameters.multifd_compression;
params->has_multifd_zlib_level = true;
params->multifd_zlib_level = s->parameters.multifd_zlib_level;
+params->has_multifd_zstd_level = true;
+params->multifd_zstd_level = s->parameters.multifd_zstd_level;
params->has_xbzrle_cache_size = true;
params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
params->has_max_postcopy_bandwidth = true;
@@ -1219,6 +1223,13 @@ static bool migrate_params_check(MigrationParameters
*params, Error **errp)
return false;
}
+if (params->has_multifd_zstd_level &&
+(params->multifd_zstd_level > 20)) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
+ "is invalid, it should be in the range of 0 to 20");
+return false;
+}
+
if (params->has_xbzrle_cache_size &&
(params->xbzrle_cache_size < qemu_target_page_size() ||
!is_power_of_2(params->xbzrle_cache_size))) {
@@ -2274,6 +2285,15 @@ int migrate_multifd_zlib_level(void)
return s->parameters.multifd_zlib_level;
}
+int migrate_multifd_zstd_level(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->parameters.multifd_zstd_level;
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
@@ -3567,6 +3587,9 @@ static Property migration_properties[] = {
DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
parameters.multifd_zlib_level,
DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
+DEFINE_PROP_UINT8("multifd-zstd-level", MigrationState,
+ parameters.multifd_zstd_level,
+ DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL),
DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
parameters.xbzrle_cache_size,
DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
@@ -3659,6 +3682,7 @@ static void migration_instance_init(Object *obj)
params->has_multifd_channels = true;
params->has_multifd_compression = true;
params->has_multifd_zlib_level = true;
+params->has_multifd_zstd_level = true;
params->has_xbzrle_cache_size = true;
params->has_max_postcopy_bandwidth = true;
params->has_max_cpu_throttle = true;
diff --git a/migration/migration.h b/migration/migration.h
index c363ef9334..507284e563 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -302,6 +302,7 @@ bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
MultiFDCompression migrate_multifd_compression(void);
int migrate_multifd_zlib_level(void);
+int migrate_multifd_zstd_level(void);
int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 8a39dff589..f9f2c4446a 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1840,6 +1840,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict
*qdict)
p->has_multifd_zlib_level = true;
visit_type_int(v, param, >multifd_zlib_level, );
break;
+case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
+p->has_multifd_zstd_level = true;
+visit_type_int(v, param, >multifd_zstd_level, );
+break;
case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
p->has_xbzrle_cache_size = true;
visit_type_size(v, param, _size, );
diff --git a/qapi/migration.json b/qapi/migration.json
index 860609231b..1fba695e2e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -610,6 +610,13 @@
# will consume more CPU.
# Defaults to 1. (Since 5.0)
#
+# @multifd-zstd-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 20, where 0 means no compression, 1 means the best
+# compression speed, and 20 means best compression ratio which
+#
[PATCH v6 8/8] multifd: Add zstd compression multifd support
Signed-off-by: Juan Quintela
Acked-by: Markus Armbruster
Reviewed-by: Dr. David Alan Gilbert
---
hw/core/qdev-properties.c| 2 +-
migration/Makefile.objs | 1 +
migration/multifd-zstd.c | 339 +++
migration/multifd.h | 1 +
migration/ram.c | 1 -
qapi/migration.json | 4 +-
tests/qtest/migration-test.c | 10 ++
7 files changed, 355 insertions(+), 3 deletions(-)
create mode 100644 migration/multifd-zstd.c
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index db2a7abfb2..2047114fca 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -645,7 +645,7 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
const PropertyInfo qdev_prop_multifd_compression = {
.name = "MultiFDCompression",
.description = "multifd_compression values, "
- "none/zlib",
+ "none/zlib/zstd",
.enum_table = _lookup,
.get = get_enum,
.set = set_enum,
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 0308caa5c5..0fc619e380 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -9,6 +9,7 @@ common-obj-y += qjson.o
common-obj-y += block-dirty-bitmap.o
common-obj-y += multifd.o
common-obj-y += multifd-zlib.o
+common-obj-$(CONFIG_ZSTD) += multifd-zstd.o
common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
new file mode 100644
index 00..693bddf8c9
--- /dev/null
+++ b/migration/multifd-zstd.c
@@ -0,0 +1,339 @@
+/*
+ * Multifd zlib compression implementation
+ *
+ * Copyright (c) 2020 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include
+#include "qemu/rcu.h"
+#include "exec/target_page.h"
+#include "qapi/error.h"
+#include "migration.h"
+#include "trace.h"
+#include "multifd.h"
+
+struct zstd_data {
+/* stream for compression */
+ZSTD_CStream *zcs;
+/* stream for decompression */
+ZSTD_DStream *zds;
+/* buffers */
+ZSTD_inBuffer in;
+ZSTD_outBuffer out;
+/* compressed buffer */
+uint8_t *zbuff;
+/* size of compressed buffer */
+uint32_t zbuff_len;
+};
+
+/* Multifd zstd compression */
+
+/**
+ * zstd_send_setup: setup send side
+ *
+ * Setup each channel with zstd compression.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int zstd_send_setup(MultiFDSendParams *p, Error **errp)
+{
+uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+struct zstd_data *z = g_new0(struct zstd_data, 1);
+int res;
+
+p->data = z;
+z->zcs = ZSTD_createCStream();
+if (!z->zcs) {
+g_free(z);
+error_setg(errp, "multifd %d: zstd createCStream failed", p->id);
+return -1;
+}
+
+res = ZSTD_initCStream(z->zcs, migrate_multifd_zstd_level());
+if (ZSTD_isError(res)) {
+ZSTD_freeCStream(z->zcs);
+g_free(z);
+error_setg(errp, "multifd %d: initCStream failed with error %s",
+ p->id, ZSTD_getErrorName(res));
+return -1;
+}
+/* We will never have more than page_count pages */
+z->zbuff_len = page_count * qemu_target_page_size();
+z->zbuff_len *= 2;
+z->zbuff = g_try_malloc(z->zbuff_len);
+if (!z->zbuff) {
+ZSTD_freeCStream(z->zcs);
+g_free(z);
+error_setg(errp, "multifd %d: out of memory for zbuff", p->id);
+return -1;
+}
+return 0;
+}
+
+/**
+ * zstd_send_cleanup: cleanup send side
+ *
+ * Close the channel and return memory.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void zstd_send_cleanup(MultiFDSendParams *p, Error **errp)
+{
+struct zstd_data *z = p->data;
+
+ZSTD_freeCStream(z->zcs);
+z->zcs = NULL;
+g_free(z->zbuff);
+z->zbuff = NULL;
+g_free(p->data);
+p->data = NULL;
+}
+
+/**
+ * zstd_send_prepare: prepare date to be able to send
+ *
+ * Create a compressed buffer with all the pages that we are going to
+ * send.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ */
+static int zstd_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+struct iovec *iov = p->pages->iov;
+struct zstd_data *z = p->data;
+int ret;
+uint32_t i;
+
+z->out.dst = z->zbuff;
+z->out.size = z->zbuff_len;
+z->out.pos = 0;
+
+for (i = 0; i < used; i++) {
+ZSTD_EndDirective flush = ZSTD_e_continue;
+
+if (i == used - 1) {
+flush = ZSTD_e_flush;
+}
+z->in.src = iov[i].iov_base;
+z->in.size = iov[i].iov_len;
+z->in.pos = 0;
+
+/*
+ * Welcome to compressStream2 semantics
+
Re: [PATCH] uapi: fix userspace breakage, use __BITS_PER_LONG for swap
On Thu, Feb 13, 2020 at 03:21:47PM +0100, Christian Borntraeger wrote:
> QEMU has a funny new build error message when I use the upstream kernel
> headers:
>
> CC block/file-posix.o
> In file included from /home/cborntra/REPOS/qemu/include/qemu/timer.h:4,
> from
> /home/cborntra/REPOS/qemu/include/qemu/timed-average.h:29,
> from /home/cborntra/REPOS/qemu/include/block/accounting.h:28,
> from /home/cborntra/REPOS/qemu/include/block/block_int.h:27,
> from /home/cborntra/REPOS/qemu/block/file-posix.c:30:
> /usr/include/linux/swab.h: In function ‘__swab’:
> /home/cborntra/REPOS/qemu/include/qemu/bitops.h:20:34: error: "sizeof" is not
> defined, evaluates to 0 [-Werror=undef]
>20 | #define BITS_PER_LONG (sizeof (unsigned long) *
> BITS_PER_BYTE)
> | ^~
> /home/cborntra/REPOS/qemu/include/qemu/bitops.h:20:41: error: missing binary
> operator before token "("
>20 | #define BITS_PER_LONG (sizeof (unsigned long) *
> BITS_PER_BYTE)
> | ^
> cc1: all warnings being treated as errors
> make: *** [/home/cborntra/REPOS/qemu/rules.mak:69: block/file-posix.o] Error 1
> rm tests/qemu-iotests/socket_scm_helper.o
>
> This was triggered by commit d5767057c9a ("uapi: rename ext2_swab() to swab()
> and share globally in swab.h")
> This patch is doing
> +#include
> but it uses BITS_PER_LONG.
>
> The kernel file asm/bitsperlong.h provide only __BITS_PER_LONG.
>
> Let us use the __ variant in swap.h
> Fixes: d5767057c9a ("uapi: rename ext2_swab() to swab() and share globally in
> swab.h")
> Cc: Yury Norov
> Cc: Allison Randal
> Cc: Joe Perches
> Cc: Thomas Gleixner
> Cc: William Breathitt Gray
> Signed-off-by: Christian Borntraeger
> ---
> include/uapi/linux/swab.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
> index fa7f97da5b76..7272f85d6d6a 100644
> --- a/include/uapi/linux/swab.h
> +++ b/include/uapi/linux/swab.h
> @@ -135,9 +135,9 @@ static inline __attribute_const__ __u32 __fswahb32(__u32
> val)
>
> static __always_inline unsigned long __swab(const unsigned long y)
> {
> -#if BITS_PER_LONG == 64
> +#if __BITS_PER_LONG == 64
> return __swab64(y);
> -#else /* BITS_PER_LONG == 32 */
> +#else /* __BITS_PER_LONG == 32 */
> return __swab32(y);
> #endif
> }
There is a patch from Torsten Hilbrich fixing this:
https://lkml.org/lkml/2020/2/12/93
[PATCH v6 3/8] multifd: Make no compression operations into its own structure
It will be used later.
Signed-off-by: Juan Quintela
---
No comp value needs to be zero.
---
migration/migration.c | 9 ++
migration/migration.h | 1 +
migration/multifd.c | 185 --
migration/multifd.h | 26 ++
migration/ram.c | 1 +
5 files changed, 214 insertions(+), 8 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index bc744d1734..eb7b0a2df0 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2245,6 +2245,15 @@ int migrate_multifd_channels(void)
return s->parameters.multifd_channels;
}
+MultiFDCompression migrate_multifd_compression(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->parameters.multifd_compression;
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 8473ddfc88..59fea02482 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -300,6 +300,7 @@ bool migrate_auto_converge(void);
bool migrate_use_multifd(void);
bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
+MultiFDCompression migrate_multifd_compression(void);
int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
diff --git a/migration/multifd.c b/migration/multifd.c
index b3e8ae9bcc..97433e5135 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -38,6 +38,134 @@ typedef struct {
uint64_t unused2[4];/* Reserved for future use */
} __attribute__((packed)) MultiFDInit_t;
+/* Multifd without compression */
+
+/**
+ * nocomp_send_setup: setup send side
+ *
+ * For no compression this function does nothing.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int nocomp_send_setup(MultiFDSendParams *p, Error **errp)
+{
+return 0;
+}
+
+/**
+ * nocomp_send_cleanup: cleanup send side
+ *
+ * For no compression this function does nothing.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
+{
+return;
+}
+
+/**
+ * nocomp_send_prepare: prepare date to be able to send
+ *
+ * For no compression we just have to calculate the size of the
+ * packet.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
+ Error **errp)
+{
+p->next_packet_size = used * qemu_target_page_size();
+p->flags |= MULTIFD_FLAG_NOCOMP;
+return 0;
+}
+
+/**
+ * nocomp_send_write: do the actual write of the data
+ *
+ * For no compression we just have to write the data.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int nocomp_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+return qio_channel_writev_all(p->c, p->pages->iov, used, errp);
+}
+
+/**
+ * nocomp_recv_setup: setup receive side
+ *
+ * For no compression this function does nothing.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
+{
+return 0;
+}
+
+/**
+ * nocomp_recv_cleanup: setup receive side
+ *
+ * For no compression this function does nothing.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void nocomp_recv_cleanup(MultiFDRecvParams *p)
+{
+}
+
+/**
+ * nocomp_recv_pages: read the data from the channel into actual pages
+ *
+ * For no compression we just need to read things into the correct place.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int nocomp_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
+{
+uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
+
+if (flags != MULTIFD_FLAG_NOCOMP) {
+error_setg(errp, "multifd %d: flags received %x flags expected %x",
+ p->id, flags, MULTIFD_FLAG_NOCOMP);
+return -1;
+}
+return qio_channel_readv_all(p->c, p->pages->iov, used, errp);
+}
+
+static MultiFDMethods multifd_nocomp_ops = {
+.send_setup = nocomp_send_setup,
+.send_cleanup = nocomp_send_cleanup,
+.send_prepare = nocomp_send_prepare,
+.send_write = nocomp_send_write,
+.recv_setup = nocomp_recv_setup,
+.recv_cleanup = nocomp_recv_cleanup,
+.recv_pages = nocomp_recv_pages
+};
+
+static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = {
+[MULTIFD_COMPRESSION_NONE] = _nocomp_ops,
+};
+
static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
{
[PATCH v6 2/8] migration: Add support for modules
So we don't have to compile everything in, or have ifdefs
Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
---
include/qemu/module.h | 2 ++
vl.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 65ba596e46..907cb5c0a5 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -40,6 +40,7 @@ static void __attribute__((constructor)) do_qemu_init_ ##
function(void)\
#endif
typedef enum {
+MODULE_INIT_MIGRATION,
MODULE_INIT_BLOCK,
MODULE_INIT_OPTS,
MODULE_INIT_QOM,
@@ -56,6 +57,7 @@ typedef enum {
#define xen_backend_init(function) module_init(function, \
MODULE_INIT_XEN_BACKEND)
#define libqos_init(function) module_init(function, MODULE_INIT_LIBQOS)
+#define migration_init(function) module_init(function, MODULE_INIT_MIGRATION)
#define block_module_load_one(lib) module_load_one("block-", lib)
#define ui_module_load_one(lib) module_load_one("ui-", lib)
diff --git a/vl.c b/vl.c
index b0ee318f99..11355f6b9a 100644
--- a/vl.c
+++ b/vl.c
@@ -2899,6 +2899,7 @@ int main(int argc, char **argv, char **envp)
qemu_init_exec_dir(argv[0]);
module_call_init(MODULE_INIT_QOM);
+module_call_init(MODULE_INIT_MIGRATION);
qemu_add_opts(_drive_opts);
qemu_add_drive_opts(_legacy_drive_opts);
--
2.24.1
[PATCH v6 0/8] Multifd Migration Compression
Based-on: <20200213132030.57757-1-quint...@redhat.com> [v6] - rebase to latest pull request - move functions that read parameters to the parameters patechs (dave) - require zstd on travis/docker/... (please review) - rename multifd_method to multifd_compression - improve documentation Please review [v5] - rebased on top of latest pull request - check zlib/zstd return codes in loops (denis suggestions) - check that we do the right thing in error cases (I corrupted the stream, make buffers too small, etc by hand) [v4] - create new parameters: multifd-zlib-level & multifd-zstd-level - use proper "conditionals" for qapi (thanks markus) - more than half of the patches moved to the migration PULL request that this series are based on - method type has moved to one int from a set of flags - fixed all reviews comments Please review. [v3] - rebased on top of upstream + previous multifd cancel series - split multifd code into its own file (multifd.[ch]) - split zstd/zlib compression methods (multifd-zstd/zlib.c) - use qemu module feauture to avoid ifdefs (my understanding is that zlib needs to be present, but we setup zstd only if it is not there or is disabled) - multifd-method: none|zlib|zstd As far as I can see, there is no easy way to convince qapi that zstd option could/couldn't be there depending on compliation flags. I ended just checking in migrate_parameters_check() if it is enabled and giving an error message otherwise. Questions: - I am "reusing" the compress-level parameter for both zstd and zlib, but it poses a problem: * zlib values: 1-9 (default: 6?) * zstd values: 1-19 (default: 3) So, what should I do: * create multifd-zstd-level and multifd-zlib-level (easier) * reuse compress-level, and change its maximum values depending on multifd-method * any other good option? Please, review. [v2] - rebase on top of previous arguments posted to the list - introduces zlib compression - introduces zstd compression Please help if you know anything about zstd/zlib compression. This puts compression on top of multifd. Advantages about current compression: - We copy all pages in a single packet and then compress the whole thing. - We reuse the compression stream for all the packets sent through the same channel. - We can select nocomp/zlib/zstd levels of compression. Please, review. Juan Quintela (8): multifd: Add multifd-compression parameter migration: Add support for modules multifd: Make no compression operations into its own structure multifd: Add multifd-zlib-level parameter multifd: Add zlib compression multifd support configure: Enable test and libs for zstd multifd: Add multifd-zstd-level parameter multifd: Add zstd compression multifd support .gitlab-ci.yml| 1 + .travis.yml | 1 + configure | 30 ++ hw/core/qdev-properties.c | 13 + include/hw/qdev-properties.h | 4 + include/qemu/module.h | 2 + migration/Makefile.objs | 2 + migration/migration.c | 70 migration/migration.h | 3 + migration/multifd-zlib.c | 325 + migration/multifd-zstd.c | 339 ++ migration/multifd.c | 191 +- migration/multifd.h | 31 ++ migration/ram.c | 2 +- monitor/hmp-cmds.c| 21 ++ qapi/migration.json | 80 - tests/docker/dockerfiles/centos7.docker | 3 +- .../dockerfiles/fedora-i386-cross.docker | 3 +- tests/docker/dockerfiles/fedora.docker| 3 +- tests/docker/dockerfiles/ubuntu.docker| 1 + tests/docker/dockerfiles/ubuntu1804.docker| 1 + tests/qtest/migration-test.c | 30 +- tests/vm/fedora | 5 +- tests/vm/freebsd | 3 + tests/vm/netbsd | 3 + tests/vm/openbsd | 3 + vl.c | 1 + 27 files changed, 1151 insertions(+), 20 deletions(-) create mode 100644 migration/multifd-zlib.c create mode 100644 migration/multifd-zstd.c -- 2.24.1
[PATCH v6 1/8] multifd: Add multifd-compression parameter
This will store the compression method to use. We start with none.
Signed-off-by: Juan Quintela
Acked-by: Markus Armbruster
Reviewed-by: Dr. David Alan Gilbert
---
Rename multifd-method to multifd-compression
---
hw/core/qdev-properties.c| 13 +
include/hw/qdev-properties.h | 4
migration/migration.c| 13 +
monitor/hmp-cmds.c | 13 +
qapi/migration.json | 30 +++---
tests/qtest/migration-test.c | 14 ++
6 files changed, 80 insertions(+), 7 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 7f93bfeb88..fa7edac020 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -8,6 +8,7 @@
#include "qapi/qmp/qerror.h"
#include "qemu/ctype.h"
#include "qemu/error-report.h"
+#include "qapi/qapi-types-migration.h"
#include "hw/block/block.h"
#include "net/hub.h"
#include "qapi/visitor.h"
@@ -639,6 +640,18 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
.set_default_value = set_default_value_enum,
};
+/* --- MultiFDCompression --- */
+
+const PropertyInfo qdev_prop_multifd_compression = {
+.name = "MultiFDCompression",
+.description = "multifd_compression values, "
+ "none",
+.enum_table = _lookup,
+.get = get_enum,
+.set = set_enum,
+.set_default_value = set_default_value_enum,
+};
+
/* --- pci address --- */
/*
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 906e697c58..f161604fb6 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -20,6 +20,7 @@ extern const PropertyInfo qdev_prop_chr;
extern const PropertyInfo qdev_prop_tpm;
extern const PropertyInfo qdev_prop_macaddr;
extern const PropertyInfo qdev_prop_on_off_auto;
+extern const PropertyInfo qdev_prop_multifd_compression;
extern const PropertyInfo qdev_prop_losttickpolicy;
extern const PropertyInfo qdev_prop_blockdev_on_error;
extern const PropertyInfo qdev_prop_bios_chs_trans;
@@ -184,6 +185,9 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
#define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \
DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_on_off_auto, OnOffAuto)
+#define DEFINE_PROP_MULTIFD_COMPRESSION(_n, _s, _f, _d) \
+DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compression, \
+ MultiFDCompression)
#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
LostTickPolicy)
diff --git a/migration/migration.c b/migration/migration.c
index 8fb68795dc..bc744d1734 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -88,6 +88,7 @@
/* The delay time (in ms) between two COLO checkpoints */
#define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
#define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
+#define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
/* Background transfer rate for postcopy, 0 means unlimited, note
* that page requests can still exceed this limit.
@@ -798,6 +799,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
**errp)
params->block_incremental = s->parameters.block_incremental;
params->has_multifd_channels = true;
params->multifd_channels = s->parameters.multifd_channels;
+params->has_multifd_compression = true;
+params->multifd_compression = s->parameters.multifd_compression;
params->has_xbzrle_cache_size = true;
params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
params->has_max_postcopy_bandwidth = true;
@@ -1315,6 +1318,9 @@ static void
migrate_params_test_apply(MigrateSetParameters *params,
if (params->has_multifd_channels) {
dest->multifd_channels = params->multifd_channels;
}
+if (params->has_multifd_compression) {
+dest->multifd_compression = params->multifd_compression;
+}
if (params->has_xbzrle_cache_size) {
dest->xbzrle_cache_size = params->xbzrle_cache_size;
}
@@ -1411,6 +1417,9 @@ static void migrate_params_apply(MigrateSetParameters
*params, Error **errp)
if (params->has_multifd_channels) {
s->parameters.multifd_channels = params->multifd_channels;
}
+if (params->has_multifd_compression) {
+s->parameters.multifd_compression = params->multifd_compression;
+}
if (params->has_xbzrle_cache_size) {
s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
xbzrle_cache_resize(params->xbzrle_cache_size, errp);
@@ -3523,6 +3532,9 @@ static Property migration_properties[] = {
DEFINE_PROP_UINT8("multifd-channels", MigrationState,
parameters.multifd_channels,
DEFAULT_MIGRATE_MULTIFD_CHANNELS),
+DEFINE_PROP_MULTIFD_COMPRESSION("multifd-compression", MigrationState,
+ parameters.multifd_compression,
Re: [PATCH v5 6/8] configure: Enable test and libs for zstd
Daniel P. Berrangé wrote: > On Wed, Jan 29, 2020 at 12:56:53PM +0100, Juan Quintela wrote: >> Signed-off-by: Juan Quintela >> Reviewed-by: Dr. David Alan Gilbert >> --- >> configure | 30 ++ >> 1 file changed, 30 insertions(+) > > This is adding a new 3rd party library to QEMU that we've not previously > built and so isn't included in any of our CI platforms. Ok. Learning how one does that. > This commit should be updating at least some of our CI platforms to > request the libzstd library installation to get CI coverage for the > latest patches in this series. > Probably the docker files, I added it in all debian/centos/fedora files that zlib-dev or xen-dev > the VM installs for FreeBSD at least, A fast google finds that library is called "zstd" and that it includes the includes (put intended) tests/vm/freebsd Once there, include it in fedora > travis and I added it to .travis.yml > gitlab CI. gitlab-ci.yml (just when we compile x86_64-softmmu) I have something like this, but net real clue how to test that I haven't broken anything: commit 59d8694dfcfc3d5ef36bc72a5c02bedaa3a6a6ec Author: Juan Quintela Date: Thu Feb 13 22:06:16 2020 +0100 Use zstd packages Signed-off-by: Juan Quintela diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c15e394f09..72f8b8aa51 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -21,6 +21,7 @@ build-system2: script: - apt-get install -y -qq libsdl2-dev libgcrypt-dev libbrlapi-dev libaio-dev libfdt-dev liblzo2-dev librdmacm-dev libibverbs-dev libibumad-dev + libzstd-dev - mkdir build - cd build - ../configure --enable-werror --target-list="tricore-softmmu unicore32-softmmu diff --git a/.travis.yml b/.travis.yml index 5887055951..dd17301f3b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -48,6 +48,7 @@ addons: - libusb-1.0-0-dev - libvdeplug-dev - libvte-2.91-dev + - libzstd-dev - sparse - uuid-dev - gcovr diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker index 562d65be9e..cdd72de7eb 100644 --- a/tests/docker/dockerfiles/centos7.docker +++ b/tests/docker/dockerfiles/centos7.docker @@ -33,6 +33,7 @@ ENV PACKAGES \ tar \ vte-devel \ xen-devel \ -zlib-devel +zlib-devel \ +libzstd-devel RUN yum install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker index 9106cf9ebe..cd16cd1bfa 100644 --- a/tests/docker/dockerfiles/fedora-i386-cross.docker +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker @@ -7,7 +7,8 @@ ENV PACKAGES \ gnutls-devel.i686 \ nettle-devel.i686 \ pixman-devel.i686 \ -zlib-devel.i686 +zlib-devel.i686 \ +libzstd-devel.i686 RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 987a3c170a..a658c0 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -92,7 +92,8 @@ ENV PACKAGES \ vte291-devel \ which \ xen-devel \ -zlib-devel +zlib-devel \ +libzstd-devel ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3 RUN dnf install -y $PACKAGES diff --git a/tests/docker/dockerfiles/ubuntu.docker b/tests/docker/dockerfiles/ubuntu.docker index 4177f33691..b6c7b41ddd 100644 --- a/tests/docker/dockerfiles/ubuntu.docker +++ b/tests/docker/dockerfiles/ubuntu.docker @@ -58,6 +58,7 @@ ENV PACKAGES flex bison \ libvdeplug-dev \ libvte-2.91-dev \ libxen-dev \ +libzstd-dev \ make \ python3-yaml \ python3-sphinx \ diff --git a/tests/docker/dockerfiles/ubuntu1804.docker b/tests/docker/dockerfiles/ubuntu1804.docker index 0766f94cf4..1efedeef99 100644 --- a/tests/docker/dockerfiles/ubuntu1804.docker +++ b/tests/docker/dockerfiles/ubuntu1804.docker @@ -44,6 +44,7 @@ ENV PACKAGES flex bison \ libvdeplug-dev \ libvte-2.91-dev \ libxen-dev \ +libzstd-dev \ make \ python3-yaml \ python3-sphinx \ diff --git a/tests/vm/fedora b/tests/vm/fedora index 4d7d6049f4..4843b4175e 100755 --- a/tests/vm/fedora +++ b/tests/vm/fedora @@ -53,7 +53,10 @@ class FedoraVM(basevm.BaseVM): # libs: audio '"pkgconfig(libpulse)"', '"pkgconfig(alsa)"', -] + +# libs: migration +'"pkgconfig(libzstd)"', +] BUILD_SCRIPT = """ set -e; diff --git a/tests/vm/freebsd b/tests/vm/freebsd index fb54334696..86770878b6 100755 --- a/tests/vm/freebsd +++ b/tests/vm/freebsd @@ -55,6 +55,9 @@ class FreeBSDVM(basevm.BaseVM): # libs: opengl "libepoxy", "mesa-libs", + +# libs: migration +"zstd", ] BUILD_SCRIPT = """ diff --git a/tests/vm/netbsd b/tests/vm/netbsd index c5069a45f4..55590f4601 100755 ---
Re: [PATCH RFC] memory: Don't allow to resize RAM while migrating
On Thu, Feb 13, 2020 at 08:42:23PM +0100, David Hildenbrand wrote: > On 13.02.20 19:32, Peter Xu wrote: > > On Thu, Feb 13, 2020 at 06:20:16PM +0100, David Hildenbrand wrote: > >> Resizing while migrating is dangerous and does not work as expected. > >> The whole migration code works on the usable_length of ram blocks and does > >> not expect this to change at random points in time. > >> > >> Precopy: The ram block size must not change on the source, after > >> ram_save_setup(), so as long as the guest is still running on the source. > >> > >> Postcopy: The ram block size must not change on the target, after > >> synchronizing the RAM block list (ram_load_precopy()). > >> > >> AFAIKS, resizing can be trigger *after* (but not during) a reset in > >> ACPI code by the guest > >> - hw/arm/virt-acpi-build.c:acpi_ram_update() > >> - hw/i386/acpi-build.c:acpi_ram_update() > > > > What can be the pre-condition of triggering this after reset? I'm > > thinking whether QEMU can be aware of this "resizing can happen" > > condition, then we could simply stop the migration from happening even > > before the resizing happens. Thanks, > > I think the condition is not known before the guest actually tries to > read the relevant memory areas (which trigger the rebuilt+resize, and > AFAIK, the new size depends on fw config done by the guest after the > reset). So it's hard to "predict". I chimmed in without much context, sorry if I'm going to ask naive questions. :) What I was asking is about why the resizing can happen. A quick read told me that it was majorly for easier extension of ROMs (firmware updates?). With that, I'm imaging a common case for memory resizing... (1) Source QEMU runs VM on old host, with old firmware (2) Migrate source QEMU to destination new host, with new and bigger firmware (3) During the migration, the ROM size on the destination will still be the old, referring to ram_load_precopy(), as long as no system reset (4) After migration finished, when the system reboots, memory resizing happens with the new and bigger firmware loaded And is this patch trying to fix/warn when there's a reboot during (3) so the new size is discovered at a wrong time? Is my understanding correct? > > In the precopy case it would be easier to abort (although, not simple > AFAIKS), in the postcopy not so easy - because you're already partially > running on the migration target. Prior to this patch, would a precopy still survive with such an accident (asked because I _feel_ like migrating a ramblock with smaller used_length to the same ramblock with bigger used_length seems to be fine?)? Or we can stop the precopy and restart. After this patch, it'll crash the source VM (_abort specified in memory_region_ram_resize()), which seems a bit more harsh? Thanks, -- Peter Xu
Re: [PATCH v2 00/30] Convert QAPI doc comments to generate rST instead of texinfo
Patchew URL: https://patchew.org/QEMU/20200213175647.17628-1-peter.mayd...@linaro.org/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === GEN qga/qapi-generated/qapi-gen No filename or title GEN docs/index.html make: *** [/tmp/qemu-test/src/rules.mak:392: docs/interop/qemu-qmp-ref.7] Error 255 make: *** Waiting for unfinished jobs CC crypto/random-gnutls.o Traceback (most recent call last): --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=09f3d51b9f4340dda72b1b268df5eab4', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-re2uhvu7/src/docker-src.2020-02-13-15.48.43.14845:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=09f3d51b9f4340dda72b1b268df5eab4 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-re2uhvu7/src' make: *** [docker-run-test-mingw@fedora] Error 2 real2m41.270s user0m7.803s The full log is available at http://patchew.org/logs/20200213175647.17628-1-peter.mayd...@linaro.org/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] qemu-doc: Clarify extent of build platform support
On Thu, Feb 13, 2020 at 09:43:34AM +0100, Markus Armbruster wrote: > Supporting a build platform beyond its end of life makes no sense. > Spell that out just to be clear. > > Signed-off-by: Markus Armbruster Thanks! Reviewed-by: Eduardo Habkost > --- > qemu-doc.texi | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/qemu-doc.texi b/qemu-doc.texi > index a1ef6b6484..33b9597b1d 100644 > --- a/qemu-doc.texi > +++ b/qemu-doc.texi > @@ -2880,10 +2880,11 @@ lifetime distros will be assumed to ship similar > software versions. > > For distributions with long-lifetime releases, the project will aim to > support > the most recent major version at all times. Support for the previous major > -version will be dropped 2 years after the new major version is released. For > -the purposes of identifying supported software versions, the project will > look > -at RHEL, Debian, Ubuntu LTS, and SLES distros. Other long-lifetime distros > will > -be assumed to ship similar software versions. > +version will be dropped 2 years after the new major version is released, > +or when it reaches ``end of life''. For the purposes of identifying > +supported software versions, the project will look at RHEL, Debian, > +Ubuntu LTS, and SLES distros. Other long-lifetime distros will be > +assumed to ship similar software versions. > > @section Windows > > -- > 2.21.1 > -- Eduardo
Re: [PATCH v5 8/8] multifd: Add zstd compression multifd support
"Dr. David Alan Gilbert" wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> Signed-off-by: Juan Quintela
>> ---
>> hw/core/qdev-properties.c| 2 +-
>> migration/Makefile.objs | 1 +
>> migration/migration.c| 9 +
>> migration/migration.h| 1 +
>> migration/multifd-zstd.c | 337 +++
>> migration/multifd.h | 2 +-
>> migration/ram.c | 1 -
>> qapi/migration.json | 4 +-
>> tests/qtest/migration-test.c | 10 ++
>> 9 files changed, 363 insertions(+), 4 deletions(-)
>> create mode 100644 migration/multifd-zstd.c
>> +res = ZSTD_initCStream(z->zcs, migrate_multifd_zstd_level());
>> +if (ZSTD_isError(res)) {
>> +ZSTD_freeCStream(z->zcs);
>> +g_free(z);
>> +error_setg(errp, "multifd %d: initCStream failed", p->id);
>
> It might be useful to print 'res' here - you seem to decode it on the
> receive side.
Fixed all callers.
>> @@ -163,6 +164,5 @@ typedef struct {
>> } MultiFDMethods;
>>
>> void multifd_register_ops(int method, MultiFDMethods *ops);
>> -
>
> Oddment.
oops. Removed the chunk.
>
>
> Reviewed-by: Dr. David Alan Gilbert
Thanks.
Re: [PATCH v2] tests/acceptance/machine_sparc_leon3: Do not run HelenOS test by default
On 2/12/20 6:36 PM, Philippe Mathieu-Daudé wrote:
The www.helenos.org server is slow and downloading the Leon3 binary
takes too long [*]. Do not include this test in the default suite.
Similarly to commit 471c97a69b:
Currently the Avocado framework does not distinct the time spent
downloading assets vs. the time spent running a test. With big
assets (like a full VM image) the tests likely fail.
This is a limitation known by the Avocado team.
Until this issue get fixed, do not run this tests automatically.
Tests can still be run setting the AVOCADO_TIMEOUT_EXPECTED
environment variable.
[*] https://travis-ci.org/stsquad/qemu/jobs/649599880#L4198
Reported-by: Alex Bennée
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Add missing staged hunk...
---
tests/acceptance/machine_sparc_leon3.py | 4
1 file changed, 4 insertions(+)
LGTM.
Reviewed-by: Wainer dos Santos Moschetta
diff --git a/tests/acceptance/machine_sparc_leon3.py
b/tests/acceptance/machine_sparc_leon3.py
index f77e210ccb..27e4717a51 100644
--- a/tests/acceptance/machine_sparc_leon3.py
+++ b/tests/acceptance/machine_sparc_leon3.py
@@ -5,6 +5,9 @@
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
+import os
+
+from avocado import skipUnless
from avocado_qemu import Test
from avocado_qemu import wait_for_console_pattern
@@ -13,6 +16,7 @@ class Leon3Machine(Test):
timeout = 60
+@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
def test_leon3_helenos_uimage(self):
"""
:avocado: tags=arch:sparc
Re: [PATCH v5 5/8] multifd: Add zlib compression multifd support
"Dr. David Alan Gilbert" wrote:
> * Juan Quintela (quint...@redhat.com) wrote:
>> Signed-off-by: Juan Quintela
>> ---
>> hw/core/qdev-properties.c| 2 +-
>> migration/Makefile.objs | 1 +
>> migration/migration.c| 9 +
>> migration/migration.h| 1 +
>> migration/multifd-zlib.c | 325 +++
>> migration/multifd.c | 6 +
>> migration/multifd.h | 4 +
>> qapi/migration.json | 3 +-
>> tests/qtest/migration-test.c | 6 +
>> 9 files changed, 355 insertions(+), 2 deletions(-)
>> create mode 100644 migration/multifd-zlib.c
>
> Coupld of really minor points below to fix up, but other than those:
>
>
> Reviewed-by: Dr. David Alan Gilbert
>
>> }
>>
>> +int migrate_multifd_zlib_level(void)
>> +{
>> +MigrationState *s;
>> +
>> +s = migrate_get_current();
>> +
>> +return s->parameters.multifd_zlib_level;
>> +}
>> +
>
> Does this belong in the previous patch?
It is used only here. Should not make any difference.
Anyways, changing it.
>
>> int migrate_use_xbzrle(void)
>> {
>> MigrationState *s;
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 3d23a0852e..95e9c196ff 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -301,6 +301,7 @@ bool migrate_use_multifd(void);
>> bool migrate_pause_before_switchover(void);
>> int migrate_multifd_channels(void);
>> MultiFDMethod migrate_multifd_method(void);
>> +int migrate_multifd_zlib_level(void);
>> +int flush = Z_NO_FLUSH;
>> +unsigned long start = zs->total_out;
>> +
>> +if (i == used - 1) {
>
> Note an extra space there.
>
Fixed, thanks.
Re: [PATCH] tests/acceptance/ppc_prep_40p: Use cdn.netbsd.org hostname
On 2/11/20 11:45 AM, Philippe Mathieu-Daudé wrote:
Use NetBSD content delivery network to get faster downloads.
Suggested-by: Kamil Rytarowski
Signed-off-by: Philippe Mathieu-Daudé
---
tests/acceptance/ppc_prep_40p.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
LGTM.
Reviewed-by: Wainer dos Santos Moschetta
diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
index efe06037ba..6729d96f5e 100644
--- a/tests/acceptance/ppc_prep_40p.py
+++ b/tests/acceptance/ppc_prep_40p.py
@@ -34,7 +34,7 @@ def test_factory_firmware_and_netbsd(self):
'7020-40p/P12H0456.IMG')
bios_hash = '1775face4e6dc27f3a6ed955ef6eb331bf817f03'
bios_path = self.fetch_asset(bios_url, asset_hash=bios_hash)
-drive_url = ('https://ftp.netbsd.org/pub/NetBSD/NetBSD-archive/'
+drive_url = ('https://cdn.netbsd.org/pub/NetBSD/NetBSD-archive/'
'NetBSD-4.0/prep/installation/floppy/generic_com0.fs')
drive_hash = 'dbcfc09912e71bd5f0d82c7c1ee43082fb596ceb'
drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash)
@@ -67,7 +67,7 @@ def test_openbios_and_netbsd(self):
:avocado: tags=arch:ppc
:avocado: tags=machine:40p
"""
-drive_url = ('https://ftp.netbsd.org/pub/NetBSD/iso/7.1.2/'
+drive_url = ('https://cdn.netbsd.org/pub/NetBSD/iso/7.1.2/'
'NetBSD-7.1.2-prep.iso')
drive_hash = 'ac6fa2707d888b36d6fa64de6e7fe48e'
drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash,
Re: [PATCH] tests/acceptance/ppc_prep_40p: Do not run NetBSD test by default
On 2/11/20 11:19 AM, Philippe Mathieu-Daudé wrote:
The ftp.netbsd.org server is slow and downloading the NetBSD ISO
takes too long. Do not include this test in the default suite.
Similarly to commit 471c97a69b:
Currently the Avocado framework does not distinct the time spent
downloading assets vs. the time spent running a test. With big
assets (like a full VM image) the tests likely fail.
This is a limitation known by the Avocado team.
Until this issue get fixed, do not run this tests automatically.
Tests can still be run setting the AVOCADO_TIMEOUT_EXPECTED
environment variable.
Reported-by: Alex Bennée
Signed-off-by: Philippe Mathieu-Daudé
---
tests/acceptance/ppc_prep_40p.py | 1 +
1 file changed, 1 insertion(+)
LGTM.
Reviewed-by: Wainer dos Santos Moschetta
diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py
index b27572f212..efe06037ba 100644
--- a/tests/acceptance/ppc_prep_40p.py
+++ b/tests/acceptance/ppc_prep_40p.py
@@ -61,6 +61,7 @@ def test_openbios_192m(self):
wait_for_console_pattern(self, '>> CPU type PowerPC,604')
@skipIf(os.getenv('CONTINUOUS_INTEGRATION'), 'Running on Travis-CI')
+@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
def test_openbios_and_netbsd(self):
"""
:avocado: tags=arch:ppc
Re: [PATCH] tests/acceptance: Add boot tests for sh4 and mips64 QEMU advent calendar images
On 2/11/20 7:42 AM, Thomas Huth wrote:
Now that we can select the second serial console in the acceptance tests
(see commit 746f244d9720 "Allow to use other serial consoles than default"),
we can also test the sh4 image from the QEMU advent calendar 2018.
And another recent commit (ec860426dfbe "Fix handling of LL/SC instructions")
fixed a problem with qemu-system-mips64, so the mips64 from the advent
calendar now works again and can be used for acceptance testing, too.
Signed-off-by: Thomas Huth
---
.travis.yml| 2 +-
tests/acceptance/boot_linux_console.py | 23 +--
2 files changed, 22 insertions(+), 3 deletions(-)
This change looks good.
Reviewed-by: Wainer dos Santos Moschetta
diff --git a/.travis.yml b/.travis.yml
index 5887055951..71a0097878 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -313,7 +313,7 @@ matrix:
# Acceptance (Functional) tests
- name: "GCC check-acceptance"
env:
--
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
+-
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
- TEST_CMD="make check-acceptance"
after_script:
- python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for
t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
diff --git a/tests/acceptance/boot_linux_console.py
b/tests/acceptance/boot_linux_console.py
index 34d37eba3b..a38ee004b1 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -591,12 +591,12 @@ class BootLinuxConsole(Test):
console_pattern = 'No filesystem could mount root'
self.wait_for_console_pattern(console_pattern)
-def do_test_advcal_2018(self, day, tar_hash, kernel_name):
+def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
tar_url = ('https://www.qemu-advent-calendar.org'
'/2018/download/day' + day + '.tar.xz')
file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
archive.extract(file_path, self.workdir)
-self.vm.set_console()
+self.vm.set_console(console_index=console)
self.vm.add_args('-kernel',
self.workdir + '/day' + day + '/' + kernel_name)
self.vm.launch()
@@ -670,6 +670,25 @@ class BootLinuxConsole(Test):
self.vm.add_args('-M', 'graphics=off')
self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')
+def test_mips64_malta(self):
+"""
+:avocado: tags=arch:mips64
+:avocado: tags=machine:malta
+:avocado: tags=endian:big
+"""
+tar_hash = '81b030201ec3f28cb1925297f6017d3a20d7ced5'
+self.vm.add_args('-hda', self.workdir + '/day22/' + 'ri-li.qcow2',
+ '-append', 'root=/dev/hda')
+self.do_test_advcal_2018('22', tar_hash, 'vmlinux')
+
+def test_sh4_r2d(self):
+"""
+:avocado: tags=arch:sh4
+:avocado: tags=machine:r2d
+"""
+tar_hash = 'fe06a4fd8ccbf2e27928d64472939d47829d4c7e'
+self.do_test_advcal_2018('09', tar_hash, 'zImage', console=1)
+
def test_sparc_ss20(self):
"""
:avocado: tags=arch:sparc
Re: [PATCH v12 Kernel 4/7] vfio iommu: Implementation of ioctl to for dirty pages tracking.
+static int vfio_iova_dirty_bitmap(struct vfio_iommu *iommu, dma_addr_t iova,
+ size_t size, uint64_t pgsize,
+ unsigned char __user *bitmap)
+{
+ struct vfio_dma *dma;
+ dma_addr_t i = iova, iova_limit;
+ unsigned int bsize, nbits = 0, l = 0;
+ unsigned long pgshift = __ffs(pgsize);
+
+ while ((dma = vfio_find_dma(iommu, i, pgsize))) {
+ int ret, j;
+ unsigned int npages = 0, shift = 0;
+ unsigned char temp = 0;
+
+ /* mark all pages dirty if all pages are pinned and mapped. */
+ if (dma->iommu_mapped) {
+ iova_limit = min(dma->iova + dma->size, iova + size);
+ npages = iova_limit/pgsize;
+ bitmap_set(dma->bitmap, 0, npages);
npages is derived from iova_limit, which is the number of bits to set
dirty relative to the first requested iova, not iova zero, ie. the set
of dirty bits is offset from those requested unless iova == dma->iova.
Right, fixing.
Also I hope dma->bitmap was actually allocated. Not only does the
START error path potentially leave dirty tracking enabled without all
the bitmap allocated, when does the bitmap get allocated for a new
vfio_dma when dirty tracking is enabled? Seems it only occurs if a
vpfn gets marked dirty.
Right.
Fixing error paths.
+ } else if (dma->bitmap) {
+ struct rb_node *n = rb_first(>pfn_list);
+ bool found = false;
+
+ for (; n; n = rb_next(n)) {
+ struct vfio_pfn *vpfn = rb_entry(n,
+ struct vfio_pfn, node);
+ if (vpfn->iova >= i) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ i += dma->size;
+ continue;
+ }
+
+ for (; n; n = rb_next(n)) {
+ unsigned int s;
+ struct vfio_pfn *vpfn = rb_entry(n,
+ struct vfio_pfn, node);
+
+ if (vpfn->iova >= iova + size)
+ break;
+
+ s = (vpfn->iova - dma->iova) >> pgshift;
+ bitmap_set(dma->bitmap, s, 1);
+
+ iova_limit = vpfn->iova + pgsize;
+ }
+ npages = iova_limit/pgsize;
Isn't iova_limit potentially uninitialized here? For example, if our
vfio_dma covers {0,8192} and we ask for the bitmap of {0,4096} and
there's a vpfn at {4096,8192}. I think that means vpfn->iova >= i
(4096 >= 0), so we break with found = true, then we test 4096 >= 0 +
4096 and break, and npages = /pgsize.
Right, Fixing it.
+ }
+
+ bsize = dirty_bitmap_bytes(npages);
+ shift = nbits % BITS_PER_BYTE;
+
+ if (npages && shift) {
+ l--;
+ if (!access_ok((void __user *)bitmap + l,
+ sizeof(unsigned char)))
+ return -EINVAL;
+
+ ret = __get_user(temp, bitmap + l);
I don't understand why we care to get the user's bitmap, are we trying
to leave whatever garbage they might have set in it and only also set
the dirty bits? That seems unnecessary.
Suppose dma mapped ranges are {start, size}:
{0, 0xa000}, {0xa000, 0x1}
Bitmap asked from 0 - 0x1. Say suppose all pages are dirty.
Then in first iteration for dma {0,0xa000} there are 10 pages, so 10
bits are set, put_user() happens for 2 bytes, (0011 b).
In second iteration for dma {0xa000, 0x1} there are 6 pages and
these bits should be appended to previous byte. So get_user() that byte,
then shift-OR rest of the bitmap, result should be: ( b)
Without get_user() and shift-OR, resulting bitmap would be
11 0011 b which would be wrong.
Seems like if we use a put_user() approach then we should look for
adjacent vfio_dmas within the same byte/word/dword before we push it to
the user to avoid this sort of inefficiency.
Won't that add more complication to logic?
Also why do we need these access_ok() checks when we already checked
the range at the start of the ioctl?
Since pointer is updated runtime here, better to check that pointer
before using that pointer.
Sorry, I still don't understand this, we check access_ok() with a
pointer and a length, therefore as long as we're incrementing the
pointer within that length, why do we need to retest?
Ideally
