A mixed bag, all bug fixes or similar small stuff.

The following changes since commit 19eb2d4e736dc895f31fbd6b520e514f10cc08e0:

Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2019-05-07 10:43:32 +0100)

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20190507

for you to fetch changes up to 63159601fb3e396b28da14cbb71e50ed3f5a0331:

target/arm: Stop using variable length array in dc_zva (2019-05-07 12:55:04 +0100)

* Stop using variable length array in dc_zva

* Implement M-profile XPSR GE bits

* Don't enable ARMV7M_EXCP_DEBUG from reset

* armv7m_nvic: NS BFAR and BFSR are RAZ/WI if BFHFNMINS == 0

* armv7m_nvic: Check subpriority in nvic_recompute_state_secure()

* fix various minor issues to allow building for Windows-on-ARM64

* aspeed: Set SDRAM size

* Allow system registers for KVM guests to be changed by QEMU code

* raspi: Diagnose requests for too much RAM

* virt: Support firmware configuration with -blockdev

Cao Jiaxi (4):

QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets

qga: Fix mingw compilation warnings on enum conversion

util/cacheinfo: Use uint64_t on LLP64 model to satisfy Windows ARM64

osdep: Fix mingw compilation regarding stdio formats

Joel Stanley (1):

arm: aspeed: Set SDRAM size

Markus Armbruster (3):

pc: Rearrange pc_system_firmware_init()'s legacy -drive loop

pflash_cfi01: New pflash_cfi01_legacy_drive()

hw/arm/virt: Support firmware configuration with -blockdev

Peter Maydell (7):

hw/arm/raspi: Diagnose requests for too much RAM

arm: Allow system registers for KVM guests to be changed by QEMU code

hw/arm/armv7m_nvic: Check subpriority in nvic_recompute_state_secure()

hw/intc/armv7m_nvic: NS BFAR and BFSR are RAZ/WI if BFHFNMINS == 0

hw/intc/armv7m_nvic: Don't enable ARMV7M_EXCP_DEBUG from reset

target/arm: Implement XPSR GE bits

target/arm: Stop using variable length array in dc_zva

contrib/libvhost-user/libvhost-user.h | 2 +-

include/hw/arm/aspeed.h | 1 +

include/hw/arm/virt.h | 2 +

include/hw/block/flash.h | 1 +

include/qemu/compiler.h | 2 +-

include/qemu/osdep.h | 10 +-

scripts/cocci-macro-file.h | 7 +-

target/arm/cpu.h | 13 ++-

hw/arm/aspeed.c | 8 ++

hw/arm/raspi.c | 7 ++

hw/arm/virt.c | 202 ++++++++++++++++++++++------------

hw/block/pflash_cfi01.c | 28 +++++

hw/i386/pc_sysfw.c | 18 +--

hw/intc/armv7m_nvic.c | 40 ++++++-

qga/commands-win32.c | 2 +-

target/arm/helper.c | 47 +++++++-

target/arm/kvm.c | 8 ++

target/arm/kvm32.c | 20 +---

target/arm/kvm64.c | 2 +

target/arm/machine.c | 2 +-

util/cacheinfo.c | 2 +-

21 files changed, 294 insertions(+), 130 deletions(-)

From: Markus Armbruster <armbru@redhat.com>

include/hw/block/flash.h | 1 +

hw/block/pflash_cfi01.c | 28 ++++++++++++++++++++++++++++

hw/i386/pc_sysfw.c | 16 ++--------------

3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h

--- a/include/hw/block/flash.h

+++ b/include/hw/block/flash.h

@@ -XXX,XX +XXX,XX @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base,

int be);

BlockBackend *pflash_cfi01_get_blk(PFlashCFI01 *fl);

MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl);

+void pflash_cfi01_legacy_drive(PFlashCFI01 *dev, DriveInfo *dinfo);

/* pflash_cfi02.c */

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/pflash_cfi01.c

+++ b/hw/block/pflash_cfi01.c

#include "qapi/error.h"

#include "qemu/timer.h"

#include "qemu/bitops.h"

+#include "qemu/error-report.h"

#include "qemu/host-utils.h"

#include "qemu/log.h"

+#include "qemu/option.h"

#include "hw/sysbus.h"

+#include "sysemu/blockdev.h"

#include "sysemu/sysemu.h"

#include "trace.h"

@@ -XXX,XX +XXX,XX @@ MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl)

return &fl->mem;

+/*

+ * Handle -drive if=pflash for machines that use properties.

+ * If @dinfo is null, do nothing.

+ * Else if @fl's property "drive" is already set, fatal error.

+ * Else set it to the BlockBackend with @dinfo.

+ */

+void pflash_cfi01_legacy_drive(PFlashCFI01 *fl, DriveInfo *dinfo)

+ Location loc;

+ if (!dinfo) {

+ return;

+ }

+ loc_push_none(&loc);

+ qemu_opts_loc_restore(dinfo->opts);

+ if (fl->blk) {

+ error_report("clashes with -machine");

+ exit(1);

+ }

+ qdev_prop_set_drive(DEVICE(fl), "drive",

+ blk_by_legacy_dinfo(dinfo), &error_fatal);

+ loc_pop(&loc);

static void postload_update_cb(void *opaque, int running, RunState state)

PFlashCFI01 *pfl = opaque;

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/i386/pc_sysfw.c

+++ b/hw/i386/pc_sysfw.c

@@ -XXX,XX +XXX,XX @@ void pc_system_firmware_init(PCMachineState *pcms,

{

PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);

int i;

- DriveInfo *pflash_drv;

BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)];

- Location loc;

if (!pcmc->pci_enabled) {

old_pc_system_rom_init(rom_memory, true);

@@ -XXX,XX +XXX,XX @@ void pc_system_firmware_init(PCMachineState *pcms,

/* Map legacy -drive if=pflash to machine properties */

for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {

- pflash_drv = drive_get(IF_PFLASH, 0, i);

- if (pflash_drv) {

- loc_push_none(&loc);

- qemu_opts_loc_restore(pflash_drv->opts);

- if (pflash_cfi01_get_blk(pcms->flash[i])) {

- error_report("clashes with -machine");

- exit(1);

- }

- qdev_prop_set_drive(DEVICE(pcms->flash[i]), "drive",

- blk_by_legacy_dinfo(pflash_drv), &error_fatal);

- loc_pop(&loc);

- }

+ pflash_cfi01_legacy_drive(pcms->flash[i],

+ drive_get(IF_PFLASH, 0, i));

pflash_blk[i] = pflash_cfi01_get_blk(pcms->flash[i]);

2.20.1

From: Markus Armbruster <armbru@redhat.com>

The ARM virt machines put firmware in flash memory. To configure it,

you use -drive if=pflash,unit=0,... and optionally -drive

if=pflash,unit=1,...

Why two -drive? This permits setting up one part of the flash memory

read-only, and the other part read/write. It also makes upgrading

firmware on the host easier. Below the hood, we get two separate

flash devices, because we were too lazy to improve our flash device

models to support sector protection.

The problem at hand is to do the same with -blockdev somehow, as one

more step towards deprecating -drive.

We recently solved this problem for x86 PC machines, in commit

ebc29e1beab. See the commit message for design rationale.

This commit solves it for ARM virt basically the same way: new machine

properties pflash0, pflash1 forward to the onboard flash devices'

properties. Requires creating the onboard devices in the

.instance_init() method virt_instance_init(). The existing code to

pick up drives defined with -drive if=pflash is replaced by code to

desugar into the machine properties.

There are a few behavioral differences, though:

* The flash devices are always present (x86: only present if

configured)

* Flash base addresses and sizes are fixed (x86: sizes depend on

images, mapped back to back below a fixed address)

* -bios configures contents of first pflash (x86: -bios configures ROM

contents)

* -bios is rejected when first pflash is also configured with -machine

pflash0=... (x86: bios is silently ignored then)

* -machine pflash1=... does not require -machine pflash0=... (x86: it

does).

The actual code is a bit simpler than for x86 mostly due to the first

two differences.

Before the patch, all the action is in create_flash(), called from the

machine's .init() method machvirt_init():

main()

machine_run_board_init()

machvirt_init()

create_flash()

create_one_flash() for flash[0]

create

configure

includes obeying -drive if=pflash,unit=0

realize

map

fall back to -bios

create_one_flash() for flash[1]

create

configure

includes obeying -drive if=pflash,unit=1

realize

map

update FDT

To make the machine properties work, we need to move device creation

to its .instance_init() method virt_instance_init().

Another complication is machvirt_init()'s computation of

@firmware_loaded: it predicts what create_flash() will do. Instead of

predicting what create_flash()'s replacement virt_firmware_init() will

do, I decided to have virt_firmware_init() return what it did.

Requires calling it a bit earlier.

Resulting call tree:

main()

current_machine = object_new()

...

virt_instance_init()

virt_flash_create()

virt_flash_create1() for flash[0]

create

configure: set defaults

become child of machine [NEW]

add machine prop pflash0 as alias for drive [NEW]

virt_flash_create1() for flash[1]

create

configure: set defaults

become child of machine [NEW]

add machine prop pflash1 as alias for drive [NEW]

for all machine props from the command line: machine_set_property()

...

property_set_alias() for machine props pflash0, pflash1

...

set_drive() for cfi.pflash01 prop drive

this is how -machine pflash0=... etc set

machine_run_board_init(current_machine);

virt_firmware_init()

pflash_cfi01_legacy_drive()

legacy -drive if=pflash,unit=0 and =1 [NEW]

virt_flash_map()

virt_flash_map1() for flash[0]

configure: num-blocks

realize

map

virt_flash_map1() for flash[1]

configure: num-blocks

realize

map

fall back to -bios

virt_flash_fdt()

update FDT

You have László to thank for making me explain this in detail.

Signed-off-by: Markus Armbruster <armbru@redhat.com>

Acked-by: Laszlo Ersek <lersek@redhat.com>

Message-id: 20190416091348.26075-4-armbru@redhat.com

include/hw/arm/virt.h | 2 +

hw/arm/virt.c | 202 +++++++++++++++++++++++++++---------------

2 files changed, 132 insertions(+), 72 deletions(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

--- a/include/hw/arm/virt.h

+++ b/include/hw/arm/virt.h

#include "qemu/notify.h"

#include "hw/boards.h"

#include "hw/arm/arm.h"

+#include "hw/block/flash.h"

#include "sysemu/kvm.h"

#include "hw/intc/arm_gicv3_common.h"

@@ -XXX,XX +XXX,XX @@ typedef struct {

Notifier machine_done;

DeviceState *platform_bus_dev;

FWCfgState *fw_cfg;

+ PFlashCFI01 *flash[2];

bool secure;

bool highmem;

bool highmem_ecam;

diff --git a/hw/arm/virt.c b/hw/arm/virt.c

--- a/hw/arm/virt.c

+++ b/hw/arm/virt.c

#include "qemu/osdep.h"

#include "qemu/units.h"

+#include "qemu/option.h"

#include "qapi/error.h"

#include "hw/sysbus.h"

#include "hw/arm/arm.h"

@@ -XXX,XX +XXX,XX @@ static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic)

}

}

-static void create_one_flash(const char *name, hwaddr flashbase,

- hwaddr flashsize, const char *file,

- MemoryRegion *sysmem)

+#define VIRT_FLASH_SECTOR_SIZE (256 * KiB)

+

+static PFlashCFI01 *virt_flash_create1(VirtMachineState *vms,

+ const char *name,

+ const char *alias_prop_name)

{

- /* Create and map a single flash device. We use the same

- * parameters as the flash devices on the Versatile Express board.

+ /*

+ * Create a single flash device. We use the same parameters as

+ * the flash devices on the Versatile Express board.

- DriveInfo *dinfo = drive_get_next(IF_PFLASH);

DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01);

- SysBusDevice *sbd = SYS_BUS_DEVICE(dev);

- const uint64_t sectorlength = 256 * 1024;

- if (dinfo) {

- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),

- &error_abort);

- }

-

- qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);

- qdev_prop_set_uint64(dev, "sector-length", sectorlength);

+ qdev_prop_set_uint64(dev, "sector-length", VIRT_FLASH_SECTOR_SIZE);

qdev_prop_set_uint8(dev, "width", 4);

qdev_prop_set_uint8(dev, "device-width", 2);

qdev_prop_set_bit(dev, "big-endian", false);

@@ -XXX,XX +XXX,XX @@ static void create_one_flash(const char *name, hwaddr flashbase,

qdev_prop_set_uint16(dev, "id2", 0x00);

qdev_prop_set_uint16(dev, "id3", 0x00);

qdev_prop_set_string(dev, "name", name);

- qdev_init_nofail(dev);

-

- memory_region_add_subregion(sysmem, flashbase,

- sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));

-

- if (file) {

- char *fn;

- int image_size;

-

- if (drive_get(IF_PFLASH, 0, 0)) {

- error_report("The contents of the first flash device may be "

- "specified with -bios or with -drive if=pflash... "

- "but you cannot use both options at once");

- exit(1);

- }

- fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);

- if (!fn) {

- error_report("Could not find ROM image '%s'", file);

- exit(1);

- }

- image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));

- g_free(fn);

- if (image_size < 0) {

- error_report("Could not load ROM image '%s'", file);

- exit(1);

- }

- }

+ object_property_add_child(OBJECT(vms), name, OBJECT(dev),

+ &error_abort);

+ object_property_add_alias(OBJECT(vms), alias_prop_name,

+ OBJECT(dev), "drive", &error_abort);

+ return PFLASH_CFI01(dev);

}

-static void create_flash(const VirtMachineState *vms,

- MemoryRegion *sysmem,

- MemoryRegion *secure_sysmem)

+static void virt_flash_create(VirtMachineState *vms)

{

- /* Create two flash devices to fill the VIRT_FLASH space in the memmap.

- * Any file passed via -bios goes in the first of these.

+ vms->flash[0] = virt_flash_create1(vms, "virt.flash0", "pflash0");

+ vms->flash[1] = virt_flash_create1(vms, "virt.flash1", "pflash1");

+}

+

+static void virt_flash_map1(PFlashCFI01 *flash,

+ hwaddr base, hwaddr size,

+ MemoryRegion *sysmem)

+{

+ DeviceState *dev = DEVICE(flash);

+

+ assert(size % VIRT_FLASH_SECTOR_SIZE == 0);

+ assert(size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);

+ qdev_prop_set_uint32(dev, "num-blocks", size / VIRT_FLASH_SECTOR_SIZE);

+ qdev_init_nofail(dev);

+

+ memory_region_add_subregion(sysmem, base,

+ sysbus_mmio_get_region(SYS_BUS_DEVICE(dev),

+ 0));

+}

+

+static void virt_flash_map(VirtMachineState *vms,

+ MemoryRegion *sysmem,

+ MemoryRegion *secure_sysmem)

+{

+ * Map two flash devices to fill the VIRT_FLASH space in the memmap.

* sysmem is the system memory space. secure_sysmem is the secure view

* of the system, and the first flash device should be made visible only

* there. The second flash device is visible to both secure and nonsecure.

@@ -XXX,XX +XXX,XX @@ static void create_flash(const VirtMachineState *vms,

*/

hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2;

hwaddr flashbase = vms->memmap[VIRT_FLASH].base;

- char *nodename;

- create_one_flash("virt.flash0", flashbase, flashsize,

- bios_name, secure_sysmem);

- create_one_flash("virt.flash1", flashbase + flashsize, flashsize,

- NULL, sysmem);

+ virt_flash_map1(vms->flash[0], flashbase, flashsize,

+ secure_sysmem);

+ virt_flash_map1(vms->flash[1], flashbase + flashsize, flashsize,

+ sysmem);

+}

+

+static void virt_flash_fdt(VirtMachineState *vms,

+ MemoryRegion *sysmem,

+ MemoryRegion *secure_sysmem)

+{

+ hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2;

+ hwaddr flashbase = vms->memmap[VIRT_FLASH].base;

+ char *nodename;

if (sysmem == secure_sysmem) {

/* Report both flash devices as a single node in the DT */

@@ -XXX,XX +XXX,XX @@ static void create_flash(const VirtMachineState *vms,

qemu_fdt_setprop_cell(vms->fdt, nodename, "bank-width", 4);

g_free(nodename);

} else {

- /* Report the devices as separate nodes so we can mark one as

+ /*

+ * Report the devices as separate nodes so we can mark one as

* only visible to the secure world.

*/

nodename = g_strdup_printf("/secflash@%" PRIx64, flashbase);

@@ -XXX,XX +XXX,XX @@ static void create_flash(const VirtMachineState *vms,

}

}

+static bool virt_firmware_init(VirtMachineState *vms,

+ MemoryRegion *sysmem,

+ MemoryRegion *secure_sysmem)

+{

+ int i;

+ BlockBackend *pflash_blk0;

+

+ /* Map legacy -drive if=pflash to machine properties */

+ for (i = 0; i < ARRAY_SIZE(vms->flash); i++) {

+ pflash_cfi01_legacy_drive(vms->flash[i],

+ drive_get(IF_PFLASH, 0, i));

+ }

+

+ virt_flash_map(vms, sysmem, secure_sysmem);

+

+ pflash_blk0 = pflash_cfi01_get_blk(vms->flash[0]);

+

+ if (bios_name) {

+ char *fname;

+ MemoryRegion *mr;

+ int image_size;

+

+ if (pflash_blk0) {

+ error_report("The contents of the first flash device may be "

+ "specified with -bios or with -drive if=pflash... "

+ "but you cannot use both options at once");

+ exit(1);

+ }

+

+ /* Fall back to -bios */

+

+ fname = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);

+ if (!fname) {

+ error_report("Could not find ROM image '%s'", bios_name);

+ exit(1);

+ }

+ mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(vms->flash[0]), 0);

+ image_size = load_image_mr(fname, mr);

+ g_free(fname);

+ if (image_size < 0) {

+ error_report("Could not load ROM image '%s'", bios_name);

+ exit(1);

+ }

+ }

+

+ return pflash_blk0 || bios_name;

+}

+

static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)

{

hwaddr base = vms->memmap[VIRT_FW_CFG].base;

@@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine)

MemoryRegion *secure_sysmem = NULL;

int n, virt_max_cpus;

MemoryRegion *ram = g_new(MemoryRegion, 1);

- bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);

+ bool firmware_loaded;

bool aarch64 = true;

/*

@@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine)

exit(1);

}

+ if (vms->secure) {

+ if (kvm_enabled()) {

+ error_report("mach-virt: KVM does not support Security extensions");

+ exit(1);

+ }

+

+ /*

+ * The Secure view of the world is the same as the NonSecure,

+ * but with a few extra devices. Create it as a container region

+ * containing the system memory at low priority; any secure-only

+ * devices go in at higher priority and take precedence.

+ */

+ secure_sysmem = g_new(MemoryRegion, 1);

+ memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",

+ UINT64_MAX);

+ memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);

+ }

+

+ firmware_loaded = virt_firmware_init(vms, sysmem,

+ secure_sysmem ?: sysmem);

+

/* If we have an EL3 boot ROM then the assumption is that it will

* implement PSCI itself, so disable QEMU's internal implementation

* so it doesn't get in the way. Instead of starting secondary

@@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine)

exit(1);

}

- if (vms->secure) {

- if (kvm_enabled()) {

- error_report("mach-virt: KVM does not support Security extensions");

- exit(1);

- }

-

- /* The Secure view of the world is the same as the NonSecure,

- * but with a few extra devices. Create it as a container region

- * containing the system memory at low priority; any secure-only

- * devices go in at higher priority and take precedence.

- */

- secure_sysmem = g_new(MemoryRegion, 1);

- memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",

- UINT64_MAX);

- memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);

- }

-

create_fdt(vms);

possible_cpus = mc->possible_cpu_arch_ids(machine);

@@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine)

&machine->device_memory->mr);

}

- create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);

+ virt_flash_fdt(vms, sysmem, secure_sysmem);

create_gic(vms, pic);

@@ -XXX,XX +XXX,XX @@ static void virt_instance_init(Object *obj)

NULL);

vms->irqmap = a15irqmap;

+

+ virt_flash_create(vms);

}

static const TypeInfo virt_machine_info = {

2.20.1

The Raspberry Pi boards have a physical memory map which does

not allow for more than 1GB of RAM. Currently if the user tries

to ask for more then we fail in a confusing way:

$ qemu-system-aarch64 --machine raspi3 -m 8G

Unexpected error in visit_type_uintN() at qapi/qapi-visit-core.c:164:

qemu-system-aarch64: Parameter 'vcram-base' expects uint32_t

Aborted (core dumped)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

hw/arm/raspi.c | 7 +++++++

1 file changed, 7 insertions(+)

+ if (machine->ram_size > 1 * GiB) {

+ error_report("Requested ram size is too large for this machine: "

+ "maximum is 1GB");

+ exit(1);

+ }

+

object_initialize(&s->soc, sizeof(s->soc),

version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);

2.20.1

At the moment the Arm implementations of kvm_arch_{get,put}_registers()

don't support having QEMU change the values of system registers

(aka coprocessor registers for AArch32). This is because although

kvm_arch_get_registers() calls write_list_to_cpustate() to

update the CPU state struct fields (so QEMU code can read the

values in the usual way), kvm_arch_put_registers() does not

call write_cpustate_to_list(), meaning that any changes to

the CPU state struct fields will not be passed back to KVM.

The rationale for this design is documented in a comment in the

AArch32 kvm_arch_put_registers() -- writing the values in the

cpregs list into the CPU state struct is "lossy" because the

write of a register might not succeed, and so if we blindly

copy the CPU state values back again we will incorrectly

change register values for the guest. The assumption was that

no QEMU code would need to write to the registers.

However, when we implemented debug support for KVM guests, we

broke that assumption: the code to handle "set the guest up

to take a breakpoint exception" does so by updating various

guest registers including ESR_EL1.

Support this by making kvm_arch_put_registers() synchronize

CPU state back into the list. We sync only those registers

where the initial write succeeds, which should be sufficient.

This commit is the same as commit 823e1b3818f9b10b824ddc which we

had to revert in commit 942f99c825fc94c8b1a4, except that the bug

which was preventing EDK2 guest firmware running has been fixed:

kvm_arm_reset_vcpu() now calls write_list_to_cpustate().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Tested-by: Eric Auger <eric.auger@redhat.com>

---

target/arm/cpu.h | 9 ++++++++-

target/arm/helper.c | 27 +++++++++++++++++++++++++--

target/arm/kvm.c | 8 ++++++++

target/arm/kvm32.c | 20 ++------------------

target/arm/kvm64.c | 2 ++

target/arm/machine.c | 2 +-

6 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ bool write_list_to_cpustate(ARMCPU *cpu);

/**

* write_cpustate_to_list:

* @cpu: ARMCPU

+ * @kvm_sync: true if this is for syncing back to KVM

*

* For each register listed in the ARMCPU cpreg_indexes list, write

* its value from the ARMCPUState structure into the cpreg_values list.

* This is used to copy info from TCG's working data structures into

* KVM or for outbound migration.

*

+ * @kvm_sync is true if we are doing this in order to sync the

+ * register state back to KVM. In this case we will only update

+ * values in the list if the previous list->cpustate sync actually

+ * successfully wrote the CPU state. Otherwise we will keep the value

+ * that is in the list.

+ *

* Returns: true if all register values were read correctly,

* false if some register was unknown or could not be read.

* Note that we do not stop early on failure -- we will attempt

* reading all registers in the list.

*/

-bool write_cpustate_to_list(ARMCPU *cpu);

+bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);

#define ARM_CPUID_TI915T 0x54029152

#define ARM_CPUID_TI925T 0x54029252

diff --git a/target/arm/helper.c b/target/arm/helper.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static bool raw_accessors_invalid(const ARMCPRegInfo *ri)

return true;

}

-bool write_cpustate_to_list(ARMCPU *cpu)

+bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)

{

/* Write the coprocessor state from cpu->env to the (index,value) list. */

int i;

@@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu)

for (i = 0; i < cpu->cpreg_array_len; i++) {

uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);

const ARMCPRegInfo *ri;

+ uint64_t newval;

ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);

if (!ri) {

@@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu)

if (ri->type & ARM_CP_NO_RAW) {

continue;

}

- cpu->cpreg_values[i] = read_raw_cp_reg(&cpu->env, ri);

+

+ newval = read_raw_cp_reg(&cpu->env, ri);

+ if (kvm_sync) {

+ /*

+ * Only sync if the previous list->cpustate sync succeeded.

+ * Rather than tracking the success/failure state for every

+ * item in the list, we just recheck "does the raw write we must

+ * have made in write_list_to_cpustate() read back OK" here.

+ */

+ uint64_t oldval = cpu->cpreg_values[i];

+

+ if (oldval == newval) {

+ continue;

+ }

+

+ write_raw_cp_reg(&cpu->env, ri, oldval);

+ if (read_raw_cp_reg(&cpu->env, ri) != oldval) {

+ continue;

+ }

+

+ write_raw_cp_reg(&cpu->env, ri, newval);

+ }

+ cpu->cpreg_values[i] = newval;

}

return ok;

}

diff --git a/target/arm/kvm.c b/target/arm/kvm.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm.c

+++ b/target/arm/kvm.c

@@ -XXX,XX +XXX,XX @@ void kvm_arm_reset_vcpu(ARMCPU *cpu)

fprintf(stderr, "write_kvmstate_to_list failed\n");

abort();

}

+ /*

+ * Sync the reset values also into the CPUState. This is necessary

+ * because the next thing we do will be a kvm_arch_put_registers()

+ * which will update the list values from the CPUState before copying

+ * the list values back to KVM. It's OK to ignore failure returns here

+ * for the same reason we do so in kvm_arch_get_registers().

+ */

+ write_list_to_cpustate(cpu);

}

/*

diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm32.c

+++ b/target/arm/kvm32.c

@@ -XXX,XX +XXX,XX @@ int kvm_arch_put_registers(CPUState *cs, int level)

return ret;

}

- /* Note that we do not call write_cpustate_to_list()

- * here, so we are only writing the tuple list back to

- * KVM. This is safe because nothing can change the

- * CPUARMState cp15 fields (in particular gdb accesses cannot)

- * and so there are no changes to sync. In fact syncing would

- * be wrong at this point: for a constant register where TCG and

- * KVM disagree about its value, the preceding write_list_to_cpustate()

- * would not have had any effect on the CPUARMState value (since the

- * register is read-only), and a write_cpustate_to_list() here would

- * then try to write the TCG value back into KVM -- this would either

- * fail or incorrectly change the value the guest sees.

- *

- * If we ever want to allow the user to modify cp15 registers via

- * the gdb stub, we would need to be more clever here (for instance

- * tracking the set of registers kvm_arch_get_registers() successfully

- * managed to update the CPUARMState with, and only allowing those

- * to be written back up into the kernel).

- */

+ write_cpustate_to_list(cpu, true);

+

if (!write_list_to_kvmstate(cpu, level)) {

return EINVAL;

}

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm64.c

+++ b/target/arm/kvm64.c

@@ -XXX,XX +XXX,XX @@ int kvm_arch_put_registers(CPUState *cs, int level)

return ret;

}

+ write_cpustate_to_list(cpu, true);

+

if (!write_list_to_kvmstate(cpu, level)) {

return EINVAL;

}

diff --git a/target/arm/machine.c b/target/arm/machine.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/machine.c

+++ b/target/arm/machine.c

@@ -XXX,XX +XXX,XX @@ static int cpu_pre_save(void *opaque)

abort();

}

} else {

- if (!write_cpustate_to_list(cpu)) {

+ if (!write_cpustate_to_list(cpu, false)) {

/* This should never fail. */

abort();

}

--

2.20.1

From: Joel Stanley <joel@jms.id.au>

We currently use Qemu's default of 128MB. As we know how much ram each

machine ships with, make it easier on users by setting a default.

It can still be overridden with -m on the command line.

Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20190503022958.1394-1-joel@jms.id.au

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

include/hw/arm/aspeed.h | 1 +

hw/arm/aspeed.c | 8 ++++++++

2 files changed, 9 insertions(+)

diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h

--- a/include/hw/arm/aspeed.h

+++ b/include/hw/arm/aspeed.h

@@ -XXX,XX +XXX,XX @@ typedef struct AspeedBoardConfig {

const char *spi_model;

uint32_t num_cs;

void (*i2c_init)(AspeedBoardState *bmc);

+ uint32_t ram;

} AspeedBoardConfig;

#define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed")

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/aspeed.c

+++ b/hw/arm/aspeed.c

@@ -XXX,XX +XXX,XX @@

#include "sysemu/block-backend.h"

#include "hw/loader.h"

#include "qemu/error-report.h"

+#include "qemu/units.h"

static struct arm_boot_info aspeed_board_binfo = {

.board_id = -1, /* device-tree-only board */

@@ -XXX,XX +XXX,XX @@ static void aspeed_machine_class_init(ObjectClass *oc, void *data)

mc->no_floppy = 1;

mc->no_cdrom = 1;

mc->no_parallel = 1;

+ if (board->ram) {

+ mc->default_ram_size = board->ram;

+ }

amc->board = board;

@@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = {

.spi_model = "mx25l25635e",

.num_cs = 1,

.i2c_init = palmetto_bmc_i2c_init,

+ .ram = 256 * MiB,

}, {

.name = MACHINE_TYPE_NAME("ast2500-evb"),

.desc = "Aspeed AST2500 EVB (ARM1176)",

@@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = {

.spi_model = "mx25l25635e",

.num_cs = 1,

.i2c_init = ast2500_evb_i2c_init,

+ .ram = 512 * MiB,

}, {

.name = MACHINE_TYPE_NAME("romulus-bmc"),

.desc = "OpenPOWER Romulus BMC (ARM1176)",

@@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = {

.spi_model = "mx66l1g45g",

.num_cs = 2,

.i2c_init = romulus_bmc_i2c_init,

+ .ram = 512 * MiB,

}, {

.name = MACHINE_TYPE_NAME("witherspoon-bmc"),

.desc = "OpenPOWER Witherspoon BMC (ARM1176)",

@@ -XXX,XX +XXX,XX @@ static const AspeedBoardConfig aspeed_boards[] = {

.spi_model = "mx66l1g45g",

.num_cs = 2,

.i2c_init = witherspoon_bmc_i2c_init,

+ .ram = 512 * MiB,

},

};

2.20.1

From: Cao Jiaxi <driver1998@foxmail.com>

gcc_struct is for x86 only, and it generates an warning on ARM64 Clang/MinGW targets.

Signed-off-by: Cao Jiaxi <driver1998@foxmail.com>

Reviewed-by: Thomas Huth <thuth@redhat.com>

Message-id: 20190503003618.10089-1-driver1998@foxmail.com

[PMM: dropped the slirp change as slirp is now a submodule]

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

contrib/libvhost-user/libvhost-user.h | 2 +-

include/qemu/compiler.h | 2 +-

scripts/cocci-macro-file.h | 7 ++++++-

3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h

--- a/contrib/libvhost-user/libvhost-user.h

+++ b/contrib/libvhost-user/libvhost-user.h

@@ -XXX,XX +XXX,XX @@ typedef struct VhostUserInflight {

uint16_t queue_size;

} VhostUserInflight;

-#if defined(_WIN32)

+#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))

# define VU_PACKED __attribute__((gcc_struct, packed))

#else

# define VU_PACKED __attribute__((packed))

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/compiler.h

+++ b/include/qemu/compiler.h

#define QEMU_SENTINEL __attribute__((sentinel))