Re: [Qemu-devel] [PATCH for-3.2 01/11] vhost-user: define conventions for vhost-user backends
Hi, > Right. The main issue is that we need to make sure only > in-tree devices are supported. Well, that is under debate right now, see: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html > vhost-user by design > is for out of tree users. It needn't be hard, > maybe it's enough to just make qemu launch these processes > as opposed to connecting to them on command line. Not sure this is a good idea, with security being one of the motivating factors to move device emulation to other processes. When libvirt launches the processes it can place them in separate sandboxes ... cheers, Gerd
Re: [Qemu-devel] [PATCH 2/2] aspeed/scu: Implement power off register
On 12/11/18 4:10 AM, Joel Stanley wrote: > This register does not exist in hardware. It is here to allow the guest > code to cause Qemu to exit when required. > > The register address chosen is unused in the emulated machines > datasheets. yes. I checked also. On the AST2600, 0x1A0 is now in the CPU scratch register range, but I don't think this is problem. Reviewed-by: Cédric Le Goater Thanks, C. > > Signed-off-by: Joel Stanley > --- > hw/misc/aspeed_scu.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c > index c8217740efc1..aa17d032ba93 100644 > --- a/hw/misc/aspeed_scu.c > +++ b/hw/misc/aspeed_scu.c > @@ -16,6 +16,7 @@ > #include "qapi/visitor.h" > #include "qemu/bitops.h" > #include "qemu/log.h" > +#include "sysemu/sysemu.h" > #include "crypto/random.h" > #include "trace.h" > > @@ -84,6 +85,7 @@ > #define SRAM_DECODE_BASE1TO_REG(0x194) > #define SRAM_DECODE_BASE2TO_REG(0x198) > #define BMC_REV TO_REG(0x19C) > +#define POWEROFF TO_REG(0x1A0) > #define BMC_DEV_ID TO_REG(0x1A4) > > #define SCU_IO_REGION_SIZE 0x1000 > @@ -264,6 +266,9 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, > uint64_t data, > } > /* Avoid assignment below, we've handled everything */ > return; > +case POWEROFF: > +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > +break; > case FREQ_CNTR_EVAL: > case VGA_SCRATCH1 ... VGA_SCRATCH8: > case RNG_DATA: >
Re: [Qemu-devel] [PATCH 1/2] aspeed: Add syscon-poweroff to guest device tree
On 12/11/18 4:10 AM, Joel Stanley wrote:
> This adds a node to the guest's device tree that allows it to cause Qemu
> to exit when the guest shuts down.
Do you think we could find a way to add the same node under U-Boot for
tests using flash images ? or could we just add the node in DT ? Would
that be a problem on real system ?
Nevertheless, this is fine.
Reviewed-by: Cédric Le Goater
Thanks,
C.
> Signed-off-by: Joel Stanley
> ---
> hw/arm/aspeed.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 515898548284..00060d44ad51 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -21,8 +21,10 @@
> #include "hw/i2c/smbus.h"
> #include "qemu/log.h"
> #include "sysemu/block-backend.h"
> +#include "sysemu/device_tree.h"
> #include "hw/loader.h"
> #include "qemu/error-report.h"
> +#include
>
> static struct arm_boot_info aspeed_board_binfo = {
> .board_id = -1, /* device-tree-only board */
> @@ -126,6 +128,36 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr
> addr, size_t rom_size,
> g_free(storage);
> }
>
> +static void fdt_add_shutdown_node(void *fdt)
> +{
> +const char *nodename = "/syscon-poweroff";
> +uint32_t phandle;
> +int offset;
> +
> +/* Find the scu phandle */
> +offset = fdt_path_offset(fdt, "/ahb/apb/syscon@1e6e2000");
> +if (offset < 0) {
> +error_report("%s couldn't find syscon, guest shutdown unavailable:
> %s",
> + __func__, fdt_strerror(offset));
> +return;
> +}
> +phandle = fdt_get_phandle(fdt, offset);
> +
> +/* Add syscon-poweroff node and use 0x1A0, an un-used SCU register */
> +qemu_fdt_add_subnode(fdt, nodename);
> +qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
> +qemu_fdt_setprop_cells(fdt, nodename, "regmap", phandle);
> +qemu_fdt_setprop_cells(fdt, nodename, "offset", 0x1A0);
> +qemu_fdt_setprop_cells(fdt, nodename, "value", 1);
> +}
> +
> +static void aspeed_board_modify_dtb(const struct arm_boot_info *binfo,
> +void *fdt)
> +{
> +fdt_add_shutdown_node(fdt);
> +}
> +
> +
> static void aspeed_board_init_flashes(AspeedSMCState *s, const char
> *flashtype,
>Error **errp)
> {
> @@ -228,6 +260,7 @@ static void aspeed_board_init(MachineState *machine,
> aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline;
> aspeed_board_binfo.ram_size = ram_size;
> aspeed_board_binfo.loader_start = sc->info->sdram_base;
> +aspeed_board_binfo.modify_dtb = aspeed_board_modify_dtb;
>
> if (cfg->i2c_init) {
> cfg->i2c_init(bmc);
>
Re: [Qemu-devel] [RFC] block: Is name of BlockBackend deprecated with -blockdev parameter?
I figure Kevin knows, but you typoed his e-mail address. I fixed it for you. Anton Kuchin writes: > Hello, > > I'm trying to switch from -drive parameter to -blockdev + -device and > having problems. Looks like with this option I have no way to set the > name of created BlockBackend, but some QMP and HMP commands are > trying to find blk with blk_by_name() and fail to locate my device > (e.g. hmp_commit, qmp_x_bloc_latency_histogram_set ...). Was it > intentional and BB names are going to be deprecated? > > This also seems to be a the case for all block devices hotplugged with > QMP as they use the same code path. > > As far as I understand all named backends are stored in > monitor_block_backends list, but I can't get what is the point of > having this list, and why parse_drive() function doesn't call > monitor_add_blk() like blockdev_init() does with -drive option. Can > someone give me a hint on this? > > I also noticed that some commands fallback to search by qdev_id or > BDS->node_name, but at the creation time (both in > bdrv_assing_node_name and monitor_add_blk) it is already checked that > names are unique across these namespaces so may be it would be useful > to introduce generic search function? > > Thanks, > Anton
[Qemu-devel] [PATCH] usb-audio: ignore usb packages with wrong size
usb packets with no payload (zero length) seem to happen in practice for
whatever reason. Add a check and skip the packet then, otherwise we'll
trigger an assert.
Reported-by: Leonardo Soares Müller
Signed-off-by: Gerd Hoffmann
---
hw/usb/dev-audio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index ee43e4914d..28ac7c5165 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -321,6 +321,9 @@ static int streambuf_put(struct streambuf *buf, USBPacket
*p)
if (!free) {
return 0;
}
+if (p->iov.size != USBAUDIO_PACKET_SIZE) {
+return 0;
+}
assert(free >= USBAUDIO_PACKET_SIZE);
usb_packet_copy(p, buf->data + (buf->prod % buf->size),
USBAUDIO_PACKET_SIZE);
--
2.9.3
[Qemu-devel] [PATCH qemu 1/3] configure/fdt: Use more strict test for libfdt version
The libfdt installed in the system is preferred to the dtc submodule by
default. The recent libfdt update added a new symbol - fdt_check_full -
and this breaks compile if there is an older libfdt installed in
the system.
This changes the test to force ./configure into using newer libfdt.
Signed-off-by: Alexey Kardashevskiy
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 0a3c6a7..e5312da 100755
--- a/configure
+++ b/configure
@@ -3880,7 +3880,7 @@ if test "$fdt" != "no" ; then
cat > $TMPC << EOF
#include
#include
-int main(void) { fdt_first_subnode(0, 0); return 0; }
+int main(void) { fdt_check_full(NULL, 0); return 0; }
EOF
if compile_prog "" "$fdt_libs" ; then
# system DTC is good - use it
--
2.17.1
[Qemu-devel] [PATCH qemu 3/3] spapr: Fix fdt warnings
The FDT blob which the spapr machine renders at reset time produces warnings like this: my-181211-154309.dts: Warning (unit_address_format): Node /memory@8000 unit name should not have leading 0s my-181211-154309.dts: Warning (unit_address_format): Node /memory@4000 unit name should not have leading 0s my-181211-154309.dts: Warning (unit_address_format): Node /memory@2000 unit name should not have leading 0s my-181211-154309.dts: Warning (unit_address_format): Node /memory@1000 unit name should not have leading 0s my-181211-154309.dts: Warning (unit_address_format): Node /memory@ unit name should not have leading 0s because TARGET_FMT_lx is defined as "%016"PRIx64. This uses simple "%lx" to suppress the warning. Since it is spapr which is always 64bit, we assume here that hwaddr is always "long". Signed-off-by: Alexey Kardashevskiy --- hw/ppc/spapr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 984bf32..be565f0 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -376,7 +376,7 @@ static int spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start, mem_reg_property[0] = cpu_to_be64(start); mem_reg_property[1] = cpu_to_be64(size); -sprintf(mem_name, "memory@" TARGET_FMT_lx, start); +sprintf(mem_name, "memory@%lx", start); off = fdt_add_subnode(fdt, 0, mem_name); _FDT(off); _FDT((fdt_setprop_string(fdt, off, "device_type", "memory"))); -- 2.17.1
[Qemu-devel] [PATCH qemu 0/3] ppc/spapr: Receive and store device tree blob from SLOF
Here is my FDT queue for sPAPR. This is based on sha1 41bcd77 Cédric Le Goater "spapr: Add a pseries-4.0 machine type". Please comment. Thanks. Alexey Kardashevskiy (3): configure/fdt: Use more strict test for libfdt version ppc/spapr: Receive and store device tree blob from SLOF spapr: Fix fdt warnings configure | 2 +- include/hw/ppc/spapr.h | 7 ++- hw/ppc/spapr.c | 33 +++-- hw/ppc/spapr_hcall.c | 42 ++ hw/ppc/trace-events| 3 +++ 5 files changed, 83 insertions(+), 4 deletions(-) -- 2.17.1
[Qemu-devel] [PATCH qemu 2/3] ppc/spapr: Receive and store device tree blob from SLOF
SLOF receives a device tree and updates it with various properties
before switching to the guest kernel and QEMU is not aware of any changes
made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
sense to pass the SLOF final device tree to QEMU to let it implement
RTAS related tasks better, such as PCI host bus adapter hotplug.
Specifially, now QEMU can find out the actual XICS phandle (for PHB
hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
assisted NMI - FWNMI).
This stores the initial DT blob in the sPAPR machine and replaces it
in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
This adds an @update_dt_enabled machine property to allow backward
migration.
SLOF already has a hypercall since
https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
Signed-off-by: Alexey Kardashevskiy
---
include/hw/ppc/spapr.h | 7 ++-
hw/ppc/spapr.c | 31 ++-
hw/ppc/spapr_hcall.c | 42 ++
hw/ppc/trace-events| 3 +++
4 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 1987640..86c90df 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -102,6 +102,7 @@ struct sPAPRMachineClass {
/*< public >*/
bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
+bool update_dt_enabled;/* enable KVMPPC_H_UPDATE_DT */
bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
bool pre_2_10_has_unused_icps;
bool legacy_irq_allocation;
@@ -138,6 +139,9 @@ struct sPAPRMachineState {
int vrma_adjust;
ssize_t rtas_size;
void *rtas_blob;
+uint32_t fdt_size;
+uint32_t fdt_initial_size;
+void *fdt_blob;
long kernel_size;
bool kernel_le;
uint32_t initrd_base;
@@ -464,7 +468,8 @@ struct sPAPRMachineState {
#define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
/* Client Architecture support */
#define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
-#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
+#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
+#define KVMPPC_HCALL_MAXKVMPPC_H_UPDATE_DT
typedef struct sPAPRDeviceTreeUpdateHeader {
uint32_t version_id;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8a18250..984bf32 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1654,7 +1654,10 @@ static void spapr_machine_reset(void)
/* Load the fdt */
qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
-g_free(fdt);
+g_free(spapr->fdt_blob);
+spapr->fdt_size = fdt_totalsize(fdt);
+spapr->fdt_initial_size = spapr->fdt_size;
+spapr->fdt_blob = fdt;
/* Set up the entry state */
spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
@@ -1908,6 +1911,27 @@ static const VMStateDescription vmstate_spapr_irq_map = {
},
};
+static bool spapr_dtb_needed(void *opaque)
+{
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
+
+return smc->update_dt_enabled;
+}
+
+static const VMStateDescription vmstate_spapr_dtb = {
+.name = "spapr_dtb",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = spapr_dtb_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
+VMSTATE_UINT32(fdt_size, sPAPRMachineState),
+VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
+ fdt_size),
+VMSTATE_END_OF_LIST()
+},
+};
+
static const VMStateDescription vmstate_spapr = {
.name = "spapr",
.version_id = 3,
@@ -1937,6 +1961,7 @@ static const VMStateDescription vmstate_spapr = {
_spapr_cap_ibs,
_spapr_irq_map,
_spapr_cap_nested_kvm_hv,
+_spapr_dtb,
NULL
}
};
@@ -3871,6 +3896,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
void *data)
hc->unplug = spapr_machine_device_unplug;
smc->dr_lmb_enabled = true;
+smc->update_dt_enabled = true;
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
mc->has_hotpluggable_cpus = true;
smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
@@ -3981,8 +4007,11 @@ static void
spapr_machine_3_1_instance_options(MachineState *machine)
static void spapr_machine_3_1_class_options(MachineClass *mc)
{
+sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
+
spapr_machine_4_0_class_options(mc);
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
+smc->update_dt_enabled = false;
}
DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index ae913d0..dfd3cf3 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1717,6 +1717,46 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU
*cpu,
args[0] = characteristics;
args[1] = behaviour;
+return H_SUCCESS;
[Qemu-devel] [Bug 1368204] Re: WinME isn't able to detect QEMU's cdrom drive and other hard drives automatically
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1368204 Title: WinME isn't able to detect QEMU's cdrom drive and other hard drives automatically Status in QEMU: Expired Bug description: On a fresh installation of Windows Millennium (WinME) in qemu, Windows Me isn't able to find the CD-ROM drive or additional hard drives other than -hda at first place. Only if i add manually an IDE controller driver in Windows ME's device manager, the CD-ROM inserted in QEMU is found. Thus an IDE controller isn't found automatically either. This shouldn't be the case. On normal real hardware, Windows ME would find at least one IDE or SCSI controller. The command line that was used is the following: sudo /usr/bin/qemu-system-i386 -hda WinME_QEMU.img -cdrom drivers.iso -boot c -no-acpi -no-hpet -soundhw sb16 -net nic -cpu pentium3 -m 256 -vga cirrus qemu's version is: qemu-system-i386 --version QEMU emulator version 2.0.0 (Debian 2.0.0+dfsg-2ubuntu1.3), Copyright (c) 2003-2008 Fabrice Bellard To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1368204/+subscriptions
Re: [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
On Tue, Dec 11, 2018 at 03:51:09AM +, xuyandong wrote: > > On Tue, Dec 11, 2018 at 02:55:43AM +, xuyandong wrote: > > > On Tue, Dec 11, 2018 at 01:47:37AM +, xuyandong wrote: > > > > > On Sat, Dec 08, 2018 at 11:58:59AM +, xuyandong wrote: > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In our test, we configured VM with several pci-bridges and > > > > > > > > > a virtio-net nic been attached with bus 4, > > > > > > > > > > > > > > > > > > After VM is startup, We ping this nic from host to judge > > > > > > > > > if it is working normally. Then, we hot add pci devices to > > > > > > > > > this VM with bus > > > > 0. > > > > > > > > > > > > > > > > > > We found the virtio-net NIC in bus 4 is not working (can > > > > > > > > > not > > > > > > > > > connect) occasionally, as it kick virtio backend failure with > > > > > > > > > error > > below: > > > > > > > > > > > > > > > > > > Unassigned mem write fc803004 = 0x1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > memory-region: pci_bridge_pci > > > > > > > > > > > > > > > > > > - (prio 0, RW): > > > > > > > > > pci_bridge_pci > > > > > > > > > > > > > > > > > > fc80-fc803fff (prio 1, RW): > > > > > > > > > virtio-pci > > > > > > > > > > > > > > > > > > fc80-fc800fff (prio 0, RW): > > > > > > > > > virtio-pci-common > > > > > > > > > > > > > > > > > > fc801000-fc801fff (prio 0, RW): > > > > > > > > > virtio-pci-isr > > > > > > > > > > > > > > > > > > fc802000-fc802fff (prio 0, RW): > > > > > > > > > virtio-pci-device > > > > > > > > > > > > > > > > > > fc803000-fc803fff (prio 0, RW): > > > > > > > > > virtio-pci-notify <- io mem unassigned > > > > > > > > > > > > > > > > > > … > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > We caught an exceptional address changing while this > > > > > > > > > problem happened, show as > > > > > > > > > follow: > > > > > > > > > > > > > > > > > > Before pci_bridge_update_mappings： > > > > > > > > > > > > > > > > > > fc00-fc1f (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fc00-fc1f > > > > > > > > > > > > > > > > > > fc20-fc3f (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fc20-fc3f > > > > > > > > > > > > > > > > > > fc40-fc5f (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fc40-fc5f > > > > > > > > > > > > > > > > > > fc60-fc7f (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fc60-fc7f > > > > > > > > > > > > > > > > > > fc80-fc9f (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fc80-fc9f > > > > > > > > > <- correct Adress Spce > > > > > > > > > > > > > > > > > > fca0-fcbf (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fca0-fcbf > > > > > > > > > > > > > > > > > > fcc0-fcdf (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fcc0-fcdf > > > > > > > > > > > > > > > > > > fce0-fcff (prio 1, RW): > > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > > fce0-fcff > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > After pci_bridge_update_mappings： > > > > > > > > > > > > > > > > > > fda0-fdbf (prio 1, RW): > > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > > fda0-fdbf > > > > > > > > > > > > > > > > > > fdc0-fddf (prio 1, RW): > > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > > fdc0-fddf > > > > > > > > > > > > > > > > > > fde0-fdff (prio 1, RW): > > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > > fde0-fdff > > > > > > > > > > > > > > > > > > fe00-fe1f (prio 1, RW): > > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > > fe00-fe1f > > > > > > > > > > > > > > > > > > fe20-fe3f (prio 1, RW): > > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > > fe20-fe3f > > > > > > > > > > > > > > > > > >
Re: [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
On Tue, Dec 11, 2018 at 03:51:09AM +, xuyandong wrote:
> > There could we a way to work around this.
> > Does below help?
>
> I am sorry to tell you, I tested this patch and it doesn't work fine.
What happens?
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index
> > 236a20eaa8..7834cac4b0 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -551,7 +551,7 @@ static void build_append_pci_bus_devices(Aml
> > *parent_scope, PCIBus *bus,
> >
> > aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
> > aml_append(method,
> > -aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check
> > */)
> > +aml_call2("DVNT", aml_name("PCIU"), aml_int(4) /* Device
> > + Check Light */)
> > );
> > aml_append(method,
> > aml_call2("DVNT", aml_name("PCID"), aml_int(3)/* Eject Request
> > */)
Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] ppc/spapr: Receive and store device tree blob from SLOF
On 10/12/2018 20:30, Greg Kurz wrote:
> On Mon, 10 Dec 2018 17:20:43 +1100
> David Gibson wrote:
>
>> On Mon, Nov 12, 2018 at 03:12:26PM +1100, Alexey Kardashevskiy wrote:
>>>
>>>
>>> On 12/11/2018 05:10, Greg Kurz wrote:
Hi Alexey,
Just a few remarks. See below.
On Thu, 8 Nov 2018 12:44:06 +1100
Alexey Kardashevskiy wrote:
> SLOF receives a device tree and updates it with various properties
> before switching to the guest kernel and QEMU is not aware of any changes
> made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
> sense to pass the SLOF final device tree to QEMU to let it implement
> RTAS related tasks better, such as PCI host bus adapter hotplug.
>
> Specifially, now QEMU can find out the actual XICS phandle (for PHB
> hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
> assisted NMI - FWNMI).
>
> This stores the initial DT blob in the sPAPR machine and replaces it
> in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
>
> This adds an @update_dt_enabled machine property to allow backward
> migration.
>
> SLOF already has a hypercall since
> https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
>
> Signed-off-by: Alexey Kardashevskiy
> ---
> include/hw/ppc/spapr.h | 7 ++-
> hw/ppc/spapr.c | 29 -
> hw/ppc/spapr_hcall.c | 32
> hw/ppc/trace-events| 2 ++
> 4 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ad4d7cfd97..f5dcaf44cb 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -100,6 +100,7 @@ struct sPAPRMachineClass {
>
> /*< public >*/
> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of
> LMBs */
> +bool update_dt_enabled;/* enable KVMPPC_H_UPDATE_DT */
> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> bool pre_2_10_has_unused_icps;
> bool legacy_irq_allocation;
> @@ -136,6 +137,9 @@ struct sPAPRMachineState {
> int vrma_adjust;
> ssize_t rtas_size;
> void *rtas_blob;
> +uint32_t fdt_size;
> +uint32_t fdt_initial_size;
I don't quite see the purpose of fdt_initial_size... it seems to be only
used to print a trace.
>>>
>>>
>>> Ah, lost in rebase. The purpose was to test if the new device tree has
>>> not grown too much.
>>>
>>>
>>>
> +void *fdt_blob;
> long kernel_size;
> bool kernel_le;
> uint32_t initrd_base;
> @@ -462,7 +466,8 @@ struct sPAPRMachineState {
> #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
> /* Client Architecture support */
> #define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
> -#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
> +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
> +#define KVMPPC_HCALL_MAXKVMPPC_H_UPDATE_DT
>
> typedef struct sPAPRDeviceTreeUpdateHeader {
> uint32_t version_id;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c08130facb..5e2d4d211c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1633,7 +1633,10 @@ static void spapr_machine_reset(void)
> /* Load the fdt */
> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> -g_free(fdt);
> +g_free(spapr->fdt_blob);
> +spapr->fdt_size = fdt_totalsize(fdt);
> +spapr->fdt_initial_size = spapr->fdt_size;
> +spapr->fdt_blob = fdt;
Hmm... It looks weird to store state in a reset handler. I'd rather zeroe
both fdt_blob and fdt_size here.
>>>
>>> The device tree is built from the reset handler and the idea is that we
>>> want to always have some tree in the machine.
>>
>> Yes, I think the approach here is fine. Otherwise when we want to
>> look up the current fdt state in RTAS calls or whatever we'd always
>> have to do
>> if (fdt_blob)
>> look up that
>> else
>> look up qemu created fdt.
>>
>
> No. We only have one fdt blob: the initial one, I'd rather
> call reset time one, or the updated one.
There is one fdt in the machine, always. Either initial or from cas.
>> Incidentally 'fdt' and 'fdt_blob' names do a terrible job of
>> distinguishing what the difference is. Renaming fdt to fdt_initial
>> (to match fdt_initial_size) and fdt_blob to fdt should make that
>> clearer.
>>
>
> As mentioned earlier in this thread, spapr->fdt_initial_size is only used
> for tracing if the received fdt blob fails fdt_check_full()...
>
> $ git grep -H fdt_initial_size
> hw/ppc/spapr.c:spapr->fdt_initial_size = spapr->fdt_size;
Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] ppc/spapr: Receive and store device tree blob from SLOF
On 10/12/2018 17:20, David Gibson wrote:
> On Mon, Nov 12, 2018 at 03:12:26PM +1100, Alexey Kardashevskiy wrote:
>>
>>
>> On 12/11/2018 05:10, Greg Kurz wrote:
>>> Hi Alexey,
>>>
>>> Just a few remarks. See below.
>>>
>>> On Thu, 8 Nov 2018 12:44:06 +1100
>>> Alexey Kardashevskiy wrote:
>>>
SLOF receives a device tree and updates it with various properties
before switching to the guest kernel and QEMU is not aware of any changes
made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
sense to pass the SLOF final device tree to QEMU to let it implement
RTAS related tasks better, such as PCI host bus adapter hotplug.
Specifially, now QEMU can find out the actual XICS phandle (for PHB
hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
assisted NMI - FWNMI).
This stores the initial DT blob in the sPAPR machine and replaces it
in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
This adds an @update_dt_enabled machine property to allow backward
migration.
SLOF already has a hypercall since
https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
Signed-off-by: Alexey Kardashevskiy
---
include/hw/ppc/spapr.h | 7 ++-
hw/ppc/spapr.c | 29 -
hw/ppc/spapr_hcall.c | 32
hw/ppc/trace-events| 2 ++
4 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ad4d7cfd97..f5dcaf44cb 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -100,6 +100,7 @@ struct sPAPRMachineClass {
/*< public >*/
bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs
*/
+bool update_dt_enabled;/* enable KVMPPC_H_UPDATE_DT */
bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
bool pre_2_10_has_unused_icps;
bool legacy_irq_allocation;
@@ -136,6 +137,9 @@ struct sPAPRMachineState {
int vrma_adjust;
ssize_t rtas_size;
void *rtas_blob;
+uint32_t fdt_size;
+uint32_t fdt_initial_size;
>>>
>>> I don't quite see the purpose of fdt_initial_size... it seems to be only
>>> used to print a trace.
>>
>>
>> Ah, lost in rebase. The purpose was to test if the new device tree has
>> not grown too much.
>>
>>
>>
>>>
+void *fdt_blob;
long kernel_size;
bool kernel_le;
uint32_t initrd_base;
@@ -462,7 +466,8 @@ struct sPAPRMachineState {
#define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
/* Client Architecture support */
#define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
-#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
+#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
+#define KVMPPC_HCALL_MAXKVMPPC_H_UPDATE_DT
typedef struct sPAPRDeviceTreeUpdateHeader {
uint32_t version_id;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c08130facb..5e2d4d211c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1633,7 +1633,10 @@ static void spapr_machine_reset(void)
/* Load the fdt */
qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
-g_free(fdt);
+g_free(spapr->fdt_blob);
+spapr->fdt_size = fdt_totalsize(fdt);
+spapr->fdt_initial_size = spapr->fdt_size;
+spapr->fdt_blob = fdt;
>>>
>>> Hmm... It looks weird to store state in a reset handler. I'd rather zeroe
>>> both fdt_blob and fdt_size here.
>>
>> The device tree is built from the reset handler and the idea is that we
>> want to always have some tree in the machine.
>
> Yes, I think the approach here is fine. Otherwise when we want to
> look up the current fdt state in RTAS calls or whatever we'd always
> have to do
> if (fdt_blob)
> look up that
> else
> look up qemu created fdt.
>
> Incidentally 'fdt' and 'fdt_blob' names do a terrible job of
> distinguishing what the difference is. Renaming fdt to fdt_initial
> (to match fdt_initial_size) and fdt_blob to fdt should make that
> clearer.
There is just one fdt in sPAPRMachineState - it is fdt_blob as it is
flattened. The "fdt" symbol above is local to spapr_machine_reset() and
when the tree is built - it is stored in fdt_blob.
--
Alexey
Re: [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
> On Tue, Dec 11, 2018 at 02:55:43AM +, xuyandong wrote: > > On Tue, Dec 11, 2018 at 01:47:37AM +, xuyandong wrote: > > > > On Sat, Dec 08, 2018 at 11:58:59AM +, xuyandong wrote: > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In our test, we configured VM with several pci-bridges and > > > > > > > > a virtio-net nic been attached with bus 4, > > > > > > > > > > > > > > > > After VM is startup, We ping this nic from host to judge > > > > > > > > if it is working normally. Then, we hot add pci devices to > > > > > > > > this VM with bus > > > 0. > > > > > > > > > > > > > > > > We found the virtio-net NIC in bus 4 is not working (can > > > > > > > > not > > > > > > > > connect) occasionally, as it kick virtio backend failure with > > > > > > > > error > below: > > > > > > > > > > > > > > > > Unassigned mem write fc803004 = 0x1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > memory-region: pci_bridge_pci > > > > > > > > > > > > > > > > - (prio 0, RW): > > > > > > > > pci_bridge_pci > > > > > > > > > > > > > > > > fc80-fc803fff (prio 1, RW): > > > > > > > > virtio-pci > > > > > > > > > > > > > > > > fc80-fc800fff (prio 0, RW): > > > > > > > > virtio-pci-common > > > > > > > > > > > > > > > > fc801000-fc801fff (prio 0, RW): > > > > > > > > virtio-pci-isr > > > > > > > > > > > > > > > > fc802000-fc802fff (prio 0, RW): > > > > > > > > virtio-pci-device > > > > > > > > > > > > > > > > fc803000-fc803fff (prio 0, RW): > > > > > > > > virtio-pci-notify <- io mem unassigned > > > > > > > > > > > > > > > > … > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > We caught an exceptional address changing while this > > > > > > > > problem happened, show as > > > > > > > > follow: > > > > > > > > > > > > > > > > Before pci_bridge_update_mappings： > > > > > > > > > > > > > > > > fc00-fc1f (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fc00-fc1f > > > > > > > > > > > > > > > > fc20-fc3f (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fc20-fc3f > > > > > > > > > > > > > > > > fc40-fc5f (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fc40-fc5f > > > > > > > > > > > > > > > > fc60-fc7f (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fc60-fc7f > > > > > > > > > > > > > > > > fc80-fc9f (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fc80-fc9f > > > > > > > > <- correct Adress Spce > > > > > > > > > > > > > > > > fca0-fcbf (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fca0-fcbf > > > > > > > > > > > > > > > > fcc0-fcdf (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fcc0-fcdf > > > > > > > > > > > > > > > > fce0-fcff (prio 1, RW): > > > > > > > > alias pci_bridge_pref_mem @pci_bridge_pci > > > > > > > > fce0-fcff > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > After pci_bridge_update_mappings： > > > > > > > > > > > > > > > > fda0-fdbf (prio 1, RW): > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > fda0-fdbf > > > > > > > > > > > > > > > > fdc0-fddf (prio 1, RW): > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > fdc0-fddf > > > > > > > > > > > > > > > > fde0-fdff (prio 1, RW): > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > fde0-fdff > > > > > > > > > > > > > > > > fe00-fe1f (prio 1, RW): > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > fe00-fe1f > > > > > > > > > > > > > > > > fe20-fe3f (prio 1, RW): > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > fe20-fe3f > > > > > > > > > > > > > > > > fe40-fe5f (prio 1, RW): > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > > fe40-fe5f > > > > > > > > > > > > > > > > fe60-fe7f (prio 1, RW): > > > > > > > > alias pci_bridge_mem @pci_bridge_pci > > > > > > > >
Re: [Qemu-devel] About QEMU BQL and dirty log switch in Migration
On Fri, 19 May 2017 at 16:10, Jay Zhou wrote:
>
> Hi Paolo and Wanpeng,
>
> On 2017/5/17 16:38, Wanpeng Li wrote:
> > 2017-05-17 15:43 GMT+08:00 Paolo Bonzini :
> >>> Recently, I have tested the performance before migration and after
> >>> migration failure
> >>> using spec cpu2006 https://www.spec.org/cpu2006/, which is a standard
> >>> performance
> >>> evaluation tool.
> >>>
> >>> These are the steps:
> >>> ==
> >>> (1) the version of kmod is 4.4.11(with slightly modified) and the
> >>> version of
> >>> qemu is 2.6.0
> >>> (with slightly modified), the kmod is applied with the following
> >>> patch
> >>>
> >>> diff --git a/source/x86/x86.c b/source/x86/x86.c
> >>> index 054a7d3..75a4bb3 100644
> >>> --- a/source/x86/x86.c
> >>> +++ b/source/x86/x86.c
> >>> @@ -8550,8 +8550,10 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
> >>> */
> >>> if ((change != KVM_MR_DELETE) &&
> >>> (old->flags & KVM_MEM_LOG_DIRTY_PAGES) &&
> >>> - !(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
> >>> - kvm_mmu_zap_collapsible_sptes(kvm, new);
> >>> + !(new->flags & KVM_MEM_LOG_DIRTY_PAGES)) {
> >>> + printk(KERN_ERR "zj make KVM_REQ_MMU_RELOAD request\n");
> >>> + kvm_make_all_cpus_request(kvm, KVM_REQ_MMU_RELOAD);
> >>> + }
> >>>
> >>> /*
> >>> * Set up write protection and/or dirty logging for the new slot.
> >>
> >> Try these modifications to the setup:
> >>
> >> 1) set up 1G hugetlbfs hugepages and use those for the guest's memory
> >>
> >> 2) test both without and with the above patch.
> >>
>
> In order to avoid random memory allocation issues, I reran the test cases:
> (1) setup: start a 4U10G VM with memory preoccupied, each vcpu is pinned to a
> pcpu respectively, these resources(memory and pcpu) allocated to VM are all
> from NUMA node 0
> (2) sequence: firstly, I run the 429.mcf of spec cpu2006 before migration, and
> get a result. And then, migration failure is constructed. At last, I run the
> test case again, and get an another result.
> (3) results:
> Host hugepages THP on(2M) THP on(2M) THP on(2M) THP on(2M)
> Patchpatch1 patch2 patch3 -
> Before migration No No No Yes
> After migration failed Yes Yes Yes No
> Largepages 67->186262->1890 95->1865 1926
> score of 429.mcf 189 188 188 189
>
> Host hugepages 1G hugepages 1G hugepages 1G hugepages 1G
> hugepages
> Patchpatch1patch2patch3-
> Before migration NoNoNoYes
> After migration failed Yes Yes Yes No
> Largepages 21212639
> score of 429.mcf 188 188 186 188
>
> Notes:
> patch1 means with "lazy collapse small sptes into large sptes" codes
> patch2 means comment out "lazy collapse small sptes into large sptes" codes
> patch3 means using kvm_make_all_cpus_request(kvm, KVM_REQ_MMU_RELOAD)
> instead of kvm_mmu_zap_collapsible_sptes(kvm, new)
>
> "Largepages" means the value of /sys/kernel/debug/kvm/largepages
>
> > In addition, we can compare /sys/kernel/debug/kvm/largepages w/ and
> > w/o the patch. IIRC, /sys/kernel/debug/kvm/largepages will drop during
> > live migration, it will keep a small value if live migration fails and
> > w/o "lazy collapse small sptes into large sptes" codes, however, it
> > will increase gradually if w/ the "lazy collapse small sptes into
> > large sptes" codes.
> >
>
> No, without the "lazy collapse small sptes into large sptes" codes,
> /sys/kernel/debug/kvm/largepages does drop during live migration,
> but it still will increase gradually if live migration fails, see the result
> above. I printed out the back trace when it increases after migration failure,
>
> [139574.369098] [] dump_stack+0x19/0x1b
> [139574.369111] [] mmu_set_spte+0x2f6/0x310 [kvm]
> [139574.369122] [] __direct_map.isra.109+0x1de/0x250 [kvm]
> [139574.369133] [] tdp_page_fault+0x246/0x280 [kvm]
> [139574.369144] [] kvm_mmu_page_fault+0x24/0x130 [kvm]
> [139574.369148] [] handle_ept_violation+0x96/0x170
> [kvm_intel]
> [139574.369153] [] vmx_handle_exit+0x299/0xbf0 [kvm_intel]
> [139574.369157] [] ? uv_bau_message_intr1+0x80/0x80
> [139574.369161] [] ? vmx_inject_irq+0xf0/0xf0 [kvm_intel]
> [139574.369172] [] vcpu_enter_guest+0x76d/0x1160 [kvm]
> [139574.369184] [] ? kvm_apic_local_deliver+0x65/0x70 [kvm]
> [139574.369196] [] kvm_arch_vcpu_ioctl_run+0xd5/0x440 [kvm]
> [139574.369205] [] kvm_vcpu_ioctl+0x2b1/0x640 [kvm]
> [139574.369209] [] ? do_futex+0x122/0x5b0
> [139574.369212] [] do_vfs_ioctl+0x2e5/0x4c0
> [139574.369223] [] ? kvm_on_user_return+0x75/0xb0 [kvm]
> [139574.369225] []
Re: [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
On Tue, Dec 11, 2018 at 02:55:43AM +, xuyandong wrote: > On Tue, Dec 11, 2018 at 01:47:37AM +, xuyandong wrote: > > > On Sat, Dec 08, 2018 at 11:58:59AM +, xuyandong wrote: > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > > > In our test, we configured VM with several pci-bridges and a > > > > > > > virtio-net nic been attached with bus 4, > > > > > > > > > > > > > > After VM is startup, We ping this nic from host to judge if it > > > > > > > is working normally. Then, we hot add pci devices to this VM with > > > > > > > bus > > 0. > > > > > > > > > > > > > > We found the virtio-net NIC in bus 4 is not working (can not > > > > > > > connect) occasionally, as it kick virtio backend failure with > > > > > > > error below: > > > > > > > > > > > > > > Unassigned mem write fc803004 = 0x1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > memory-region: pci_bridge_pci > > > > > > > > > > > > > > - (prio 0, RW): > > > > > > > pci_bridge_pci > > > > > > > > > > > > > > fc80-fc803fff (prio 1, RW): virtio-pci > > > > > > > > > > > > > > fc80-fc800fff (prio 0, RW): > > > > > > > virtio-pci-common > > > > > > > > > > > > > > fc801000-fc801fff (prio 0, RW): > > > > > > > virtio-pci-isr > > > > > > > > > > > > > > fc802000-fc802fff (prio 0, RW): > > > > > > > virtio-pci-device > > > > > > > > > > > > > > fc803000-fc803fff (prio 0, RW): > > > > > > > virtio-pci-notify <- io mem unassigned > > > > > > > > > > > > > > … > > > > > > > > > > > > > > > > > > > > > > > > > > > > We caught an exceptional address changing while this problem > > > > > > > happened, show as > > > > > > > follow: > > > > > > > > > > > > > > Before pci_bridge_update_mappings： > > > > > > > > > > > > > > fc00-fc1f (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fc00-fc1f > > > > > > > > > > > > > > fc20-fc3f (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fc20-fc3f > > > > > > > > > > > > > > fc40-fc5f (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fc40-fc5f > > > > > > > > > > > > > > fc60-fc7f (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fc60-fc7f > > > > > > > > > > > > > > fc80-fc9f (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fc80-fc9f > > > > > > > <- correct Adress Spce > > > > > > > > > > > > > > fca0-fcbf (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fca0-fcbf > > > > > > > > > > > > > > fcc0-fcdf (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fcc0-fcdf > > > > > > > > > > > > > > fce0-fcff (prio 1, RW): alias > > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > > fce0-fcff > > > > > > > > > > > > > > > > > > > > > > > > > > > > After pci_bridge_update_mappings： > > > > > > > > > > > > > > fda0-fdbf (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fda0-fdbf > > > > > > > > > > > > > > fdc0-fddf (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fdc0-fddf > > > > > > > > > > > > > > fde0-fdff (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fde0-fdff > > > > > > > > > > > > > > fe00-fe1f (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fe00-fe1f > > > > > > > > > > > > > > fe20-fe3f (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fe20-fe3f > > > > > > > > > > > > > > fe40-fe5f (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fe40-fe5f > > > > > > > > > > > > > > fe60-fe7f (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fe60-fe7f > > > > > > > > > > > > > > fe80-fe9f (prio 1, RW): alias > > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > > fe80-fe9f > > > > > > > > > > > > > >
[Qemu-devel] [PATCH 2/2] aspeed/scu: Implement power off register
This register does not exist in hardware. It is here to allow the guest code to cause Qemu to exit when required. The register address chosen is unused in the emulated machines datasheets. Signed-off-by: Joel Stanley --- hw/misc/aspeed_scu.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index c8217740efc1..aa17d032ba93 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -16,6 +16,7 @@ #include "qapi/visitor.h" #include "qemu/bitops.h" #include "qemu/log.h" +#include "sysemu/sysemu.h" #include "crypto/random.h" #include "trace.h" @@ -84,6 +85,7 @@ #define SRAM_DECODE_BASE1TO_REG(0x194) #define SRAM_DECODE_BASE2TO_REG(0x198) #define BMC_REV TO_REG(0x19C) +#define POWEROFF TO_REG(0x1A0) #define BMC_DEV_ID TO_REG(0x1A4) #define SCU_IO_REGION_SIZE 0x1000 @@ -264,6 +266,9 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data, } /* Avoid assignment below, we've handled everything */ return; +case POWEROFF: +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); +break; case FREQ_CNTR_EVAL: case VGA_SCRATCH1 ... VGA_SCRATCH8: case RNG_DATA: -- 2.19.1
[Qemu-devel] [PATCH 1/2] aspeed: Add syscon-poweroff to guest device tree
This adds a node to the guest's device tree that allows it to cause Qemu
to exit when the guest shuts down.
Signed-off-by: Joel Stanley
---
hw/arm/aspeed.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 515898548284..00060d44ad51 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -21,8 +21,10 @@
#include "hw/i2c/smbus.h"
#include "qemu/log.h"
#include "sysemu/block-backend.h"
+#include "sysemu/device_tree.h"
#include "hw/loader.h"
#include "qemu/error-report.h"
+#include
static struct arm_boot_info aspeed_board_binfo = {
.board_id = -1, /* device-tree-only board */
@@ -126,6 +128,36 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr,
size_t rom_size,
g_free(storage);
}
+static void fdt_add_shutdown_node(void *fdt)
+{
+const char *nodename = "/syscon-poweroff";
+uint32_t phandle;
+int offset;
+
+/* Find the scu phandle */
+offset = fdt_path_offset(fdt, "/ahb/apb/syscon@1e6e2000");
+if (offset < 0) {
+error_report("%s couldn't find syscon, guest shutdown unavailable: %s",
+ __func__, fdt_strerror(offset));
+return;
+}
+phandle = fdt_get_phandle(fdt, offset);
+
+/* Add syscon-poweroff node and use 0x1A0, an un-used SCU register */
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
+qemu_fdt_setprop_cells(fdt, nodename, "regmap", phandle);
+qemu_fdt_setprop_cells(fdt, nodename, "offset", 0x1A0);
+qemu_fdt_setprop_cells(fdt, nodename, "value", 1);
+}
+
+static void aspeed_board_modify_dtb(const struct arm_boot_info *binfo,
+void *fdt)
+{
+fdt_add_shutdown_node(fdt);
+}
+
+
static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
Error **errp)
{
@@ -228,6 +260,7 @@ static void aspeed_board_init(MachineState *machine,
aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline;
aspeed_board_binfo.ram_size = ram_size;
aspeed_board_binfo.loader_start = sc->info->sdram_base;
+aspeed_board_binfo.modify_dtb = aspeed_board_modify_dtb;
if (cfg->i2c_init) {
cfg->i2c_init(bmc);
--
2.19.1
[Qemu-devel] [PATCH 0/2] arm: aspeed: Allow the guest to exit
This series adds a feature to the ASPEED machine that allows the guest to cause itself to exit. It was tested with romulus-bmc and palmetto-bmc with a Linux kernel as the guest. To test: cd linux $ export ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- $ make aspeed_g4_defconfig $ ./scripts/config -e CONFIG_POWER_RESET_SYSCON_POWEROFF -e CONFIG_POWER_RESET $ make -j$(nproc) $ qemu-system-arm -M palmetto-bmc -nographic \ -kernel arch/arm/boot/zImage \ -dtb arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dtb \ -initrd ~/buildroot-arm.cpio.xz Joel Stanley (2): aspeed: Add syscon-poweroff to guest device tree aspeed/scu: Implement power off register hw/arm/aspeed.c | 33 + hw/misc/aspeed_scu.c | 5 + 2 files changed, 38 insertions(+) -- 2.19.1
Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 0/6] target/ppc: convert VMX instructions to use TCG vector operations
On Tue, 11 Dec 2018, David Gibson wrote: On Mon, Dec 10, 2018 at 09:54:51PM +0100, BALATON Zoltan wrote: Yes, I don't really know what these tests use but I think "lame" test is mostly floating point but tried with "lame_vmx" which should at least use some vector ops and "mplayer -benchmark" test is more vmx dependent based on my previous profiling and testing with hardfloat but I'm not sure. (When testing these with hardfloat I've found that lame was benefiting from hardfloat but mplayer wasn't and more VMX related functions showed up with mplayer so I assumed it's more VMX bound.) I should clarify here. When I say "floating point" above, I'm not meaning things using the regular FPU instead of the vector unit. I'm saying *anything* involving floating point calculations whether they're done in the FPU or the vector unit. OK that clarifies it. I admit I was only testing these but didn't have time to look what changed exactly. The patches here don't convert all VMX instructions to use vector TCG ops - they only convert a few, and those few are about using the vector unit for integer (and logical) operations. VMX instructions involving floating point calculations are unaffected and will still use soft-float. What I've said above about lame test being more FPU and mplayer more VMX intensive probably still holds as I've retried now on a Haswell i5 and got 1-2% difference with lame_vmx and ~6% with mplayer. That's very little improvement but if only some VMX instructions should be faster then this may make sense. These tests are not the best, maybe there are better ways to measure this but I don't know of any, Maybe the PPC softmmu should be reviewed and optimised by someone who knows it... I'm not sure there is anyone who knows it at this point. I probably know it as well as anybody, and the ppc32 code scares me. It's a crufty mess and it would be nice to clean up, but that requires someone with enough time and interest. At least this seems to be a big bottleneck in PPC emulation and one that's not being worked on (others like hardfloat and VMX while not finished and still lot to do but already there are some results but no one is looking at softmmu). I was just trying to direct some attention to that softmmu may also need some optimisation and hope someone would notice this. I have some interest but not much time these days and if it scares you what should I say. I don't even understand most of it so it would take a lot of time to even get how it works and what would need to be done. So I hope someone with more time or knowledge shows up and maybe at least provides some hints on what may need to be done. Regards, BALATON Zoltan
Re: [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
On Tue, Dec 11, 2018 at 01:47:37AM +, xuyandong wrote: > > On Sat, Dec 08, 2018 at 11:58:59AM +, xuyandong wrote: > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > In our test, we configured VM with several pci-bridges and a > > > > > > virtio-net nic been attached with bus 4, > > > > > > > > > > > > After VM is startup, We ping this nic from host to judge if it > > > > > > is working normally. Then, we hot add pci devices to this VM with > > > > > > bus > 0. > > > > > > > > > > > > We found the virtio-net NIC in bus 4 is not working (can not > > > > > > connect) occasionally, as it kick virtio backend failure with error > > > > > > below: > > > > > > > > > > > > Unassigned mem write fc803004 = 0x1 > > > > > > > > > > > > > > > > > > > > > > > > memory-region: pci_bridge_pci > > > > > > > > > > > > - (prio 0, RW): > > > > > > pci_bridge_pci > > > > > > > > > > > > fc80-fc803fff (prio 1, RW): virtio-pci > > > > > > > > > > > > fc80-fc800fff (prio 0, RW): > > > > > > virtio-pci-common > > > > > > > > > > > > fc801000-fc801fff (prio 0, RW): > > > > > > virtio-pci-isr > > > > > > > > > > > > fc802000-fc802fff (prio 0, RW): > > > > > > virtio-pci-device > > > > > > > > > > > > fc803000-fc803fff (prio 0, RW): > > > > > > virtio-pci-notify <- io mem unassigned > > > > > > > > > > > > … > > > > > > > > > > > > > > > > > > > > > > > > We caught an exceptional address changing while this problem > > > > > > happened, show as > > > > > > follow: > > > > > > > > > > > > Before pci_bridge_update_mappings： > > > > > > > > > > > > fc00-fc1f (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fc00-fc1f > > > > > > > > > > > > fc20-fc3f (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fc20-fc3f > > > > > > > > > > > > fc40-fc5f (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fc40-fc5f > > > > > > > > > > > > fc60-fc7f (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fc60-fc7f > > > > > > > > > > > > fc80-fc9f (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fc80-fc9f > > > > > > <- correct Adress Spce > > > > > > > > > > > > fca0-fcbf (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fca0-fcbf > > > > > > > > > > > > fcc0-fcdf (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fcc0-fcdf > > > > > > > > > > > > fce0-fcff (prio 1, RW): alias > > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > > fce0-fcff > > > > > > > > > > > > > > > > > > > > > > > > After pci_bridge_update_mappings： > > > > > > > > > > > > fda0-fdbf (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fda0-fdbf > > > > > > > > > > > > fdc0-fddf (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fdc0-fddf > > > > > > > > > > > > fde0-fdff (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fde0-fdff > > > > > > > > > > > > fe00-fe1f (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fe00-fe1f > > > > > > > > > > > > fe20-fe3f (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fe20-fe3f > > > > > > > > > > > > fe40-fe5f (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fe40-fe5f > > > > > > > > > > > > fe60-fe7f (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fe60-fe7f > > > > > > > > > > > > fe80-fe9f (prio 1, RW): alias > > > > > > pci_bridge_mem @pci_bridge_pci > > > > > > fe80-fe9f > > > > > > > > > > > > fc80-fc80 (prio 1, RW): alias > > > pci_bridge_pref_mem > > > > > > @pci_bridge_pci fc80-fc80 <- Exceptional > Adress > > > > > Space > > > > > > > > > > This one is empty though right? > > > > > > > > > > > > > > > > > > > > > > > We have figured out why this address becomes
Re: [Qemu-devel] [PATCH qemu] hmp: Print if memory section is registered in KVM
On 14/11/2018 02:56, Paolo Bonzini wrote:
> On 13/11/2018 15:51, Dr. David Alan Gilbert wrote:
>> * Alexey Kardashevskiy (a...@ozlabs.ru) wrote:
>>> This adds a "KVM" marker to the "into mtree -f" to tell the user if
>>> a particular memory section is registered as a KVM memory slot; useful
>>> for debugging purposes.
>>>
>>> Since it is memory sections which get registered in KVM (rather than
>>> memory regions), this only prints "KVM" for flatviews.
>>>
>>> For example:
>>> Root memory region: system
>>> -0003 (prio 0, ram): ppc_spapr.ram KVM
>>> 2020-203f (prio 1, i/o): virtio-pci
>>>
>>> Signed-off-by: Alexey Kardashevskiy
>>
>> From HMP I'm ok with this, but I wonder if we should make this
>> code more general to other accelerators and not specific to kvm?
>
> Can't this be derived from the type (ram, i/o, ...)?
Sorry for the delay with the response.
Derived what? "ramd" is usually KVM but not always if we emulate a
portion of it (a bit of MMIO space from VFIO quirks, for example), and
we could do this to "ram" as well, I just do not have an example handy.
I tried to hack it. I basically need a way to tell in
mtree_print_flatview() if a specific FlatRange is "accelerated.
Option1:
- add FlatRange to MemoryRegionSection (MRSs are always local, FRs are
permanent);
- change kvm_set_phys_mem() to store accelerator in mrs->FlatRange;
- print FlatView->FlatRange->accel name.
Pro: fast - no additional lookups;
Con: accel/kvm/kvm-all.c needs to know the FlatRange definition which is
too invasive for such a small task.
Option2:
- replace global is_kvm_memory() with AccelClass::is_memory_registered();
- define the callback for KVM;
- call that from mtree_print_flatview().
Pro: no touching FlatRange/MemoryRegionSection structures.
Con: suboptimal but we probably do not care as much.
Or have I missed something? Thanks,
>
> Paolo
>
>>
>> Dave
>>
>>> ---
>>> include/sysemu/kvm.h | 1 +
>>> accel/kvm/kvm-all.c | 7 +++
>>> memory.c | 10 ++
>>> 3 files changed, 18 insertions(+)
>>>
>>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>>> index 97d8d9d0d5..9eab5ac1b1 100644
>>> --- a/include/sysemu/kvm.h
>>> +++ b/include/sysemu/kvm.h
>>> @@ -471,6 +471,7 @@ void kvm_set_sigmask_len(KVMState *s, unsigned int
>>> sigmask_len);
>>> #if !defined(CONFIG_USER_ONLY)
>>> int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
>>> hwaddr *phys_addr);
>>> +bool is_kvm_memory(hwaddr start_addr, hwaddr size);
>>> #endif
>>>
>>> #endif /* NEED_CPU_H */
>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>>> index 4880a05399..d72808aaa6 100644
>>> --- a/accel/kvm/kvm-all.c
>>> +++ b/accel/kvm/kvm-all.c
>>> @@ -216,6 +216,13 @@ static KVMSlot
>>> *kvm_lookup_matching_slot(KVMMemoryListener *kml,
>>> return NULL;
>>> }
>>>
>>> +bool is_kvm_memory(hwaddr start_addr, hwaddr size)
>>> +{
>>> +KVMMemoryListener *kml = _state->memory_listener;
>>> +
>>> +return NULL != kvm_lookup_matching_slot(kml, start_addr, size);
>>> +}
>>> +
>>> /*
>>> * Calculate and align the start address and the size of the section.
>>> * Return the size. If the size is 0, the aligned section is empty.
>>> diff --git a/memory.c b/memory.c
>>> index 51204aa079..56c733c687 100644
>>> --- a/memory.c
>>> +++ b/memory.c
>>> @@ -2916,6 +2916,7 @@ static void mtree_print_flatview(gpointer key,
>>> gpointer value,
>>> int n = view->nr;
>>> int i;
>>> AddressSpace *as;
>>> +bool print_kvm = false;
>>>
>>> p(f, "FlatView #%d\n", fvi->counter);
>>> ++fvi->counter;
>>> @@ -2927,6 +2928,9 @@ static void mtree_print_flatview(gpointer key,
>>> gpointer value,
>>> p(f, ", alias %s", memory_region_name(as->root->alias));
>>> }
>>> p(f, "\n");
>>> +if (as == _space_memory) {
>>> +print_kvm = true;
>>> +}
>>> }
>>>
>>> p(f, " Root memory region: %s\n",
>>> @@ -2960,6 +2964,12 @@ static void mtree_print_flatview(gpointer key,
>>> gpointer value,
>>> if (fvi->owner) {
>>> mtree_print_mr_owner(p, f, mr);
>>> }
>>> +
>>> +if (kvm_enabled() && print_kvm &&
>>> +is_kvm_memory(int128_get64(range->addr.start),
>>> + MR_SIZE(range->addr.size) + 1)) {
>>> +p(f, " KVM");
>>> +}
>>> p(f, "\n");
>>> range++;
>>> }
>>> --
>>> 2.17.1
>>>
>>>
>> --
>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>>
>
--
Alexey
Re: [Qemu-devel] [PATCH v7 14/19] spapr: set the interrupt presenter at reset
On Sun, Dec 09, 2018 at 08:46:05PM +0100, Cédric Le Goater wrote:
> Currently, the interrupt presenter of the vCPU is set at realize
> time. Setting it at reset will become useful when the new machine
> supporting both interrupt modes is introduced. In this machine, the
> interrupt mode is chosen at CAS time and activated after a reset.
>
> Signed-off-by: Cédric Le Goater
Shouldn't this also remove the code which sets cpu->intc at realize
time, in order to avoid confusion?
> ---
> include/hw/ppc/spapr_cpu_core.h | 2 ++
> hw/ppc/spapr_cpu_core.c | 26 ++
> hw/ppc/spapr_irq.c | 12
> 3 files changed, 40 insertions(+)
>
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 9e2821e4b31f..fc8ea9021656 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU
> *cpu)
> return (sPAPRCPUState *)cpu->machine_data;
> }
>
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
> +
> #endif
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 1811cd48db90..529de0b6b9c8 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -398,3 +398,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
> };
>
> DEFINE_TYPES(spapr_cpu_core_type_infos)
> +
> +typedef struct ForeachFindIntCArgs {
> +const char *intc_type;
> +Object *intc;
> +} ForeachFindIntCArgs;
> +
> +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
> +{
> +ForeachFindIntCArgs *args = opaque;
> +
> +if (object_dynamic_cast(child, args->intc_type)) {
> +args->intc = child;
> +}
> +
> +return args->intc != NULL;
> +}
> +
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
> +{
> +ForeachFindIntCArgs args = { intc_type, NULL };
> +
> +object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, );
> +g_assert(args.intc);
> +
> +cpu->intc = args.intc;
> +}
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 7a0d4f529763..b423cee30e2c 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -12,6 +12,7 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "hw/ppc/spapr.h"
> +#include "hw/ppc/spapr_cpu_core.h"
> #include "hw/ppc/spapr_xive.h"
> #include "hw/ppc/xics.h"
> #include "sysemu/kvm.h"
> @@ -211,6 +212,11 @@ static int spapr_irq_post_load_xics(sPAPRMachineState
> *spapr, int version_id)
>
> static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> {
> +CPUState *cs;
> +
> +CPU_FOREACH(cs) {
> +spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> +}
> }
>
> #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
> @@ -341,6 +347,12 @@ static int spapr_irq_post_load_xive(sPAPRMachineState
> *spapr, int version_id)
>
> static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
> {
> +CPUState *cs;
> +
> +CPU_FOREACH(cs) {
> +spapr_cpu_core_set_intc(POWERPC_CPU(cs), TYPE_XIVE_TCTX);
> +}
> +
> /*
> * Set the OS CAM line of the cpu interrupt thread context. Needs
> * to come after the XiveTCTX reset handlers.
--
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: [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
On Tue, Dec 11, 2018 at 01:47:37AM +, xuyandong wrote: > On Sat, Dec 08, 2018 at 11:58:59AM +, xuyandong wrote: > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > In our test, we configured VM with several pci-bridges and a > > > > > virtio-net nic been attached with bus 4, > > > > > > > > > > After VM is startup, We ping this nic from host to judge if it is > > > > > working normally. Then, we hot add pci devices to this VM with bus 0. > > > > > > > > > > We found the virtio-net NIC in bus 4 is not working (can not > > > > > connect) occasionally, as it kick virtio backend failure with error > > > > > below: > > > > > > > > > > Unassigned mem write fc803004 = 0x1 > > > > > > > > > > > > > > > > > > > > memory-region: pci_bridge_pci > > > > > > > > > > - (prio 0, RW): pci_bridge_pci > > > > > > > > > > fc80-fc803fff (prio 1, RW): virtio-pci > > > > > > > > > > fc80-fc800fff (prio 0, RW): > > > > > virtio-pci-common > > > > > > > > > > fc801000-fc801fff (prio 0, RW): > > > > > virtio-pci-isr > > > > > > > > > > fc802000-fc802fff (prio 0, RW): > > > > > virtio-pci-device > > > > > > > > > > fc803000-fc803fff (prio 0, RW): > > > > > virtio-pci-notify <- io mem unassigned > > > > > > > > > > … > > > > > > > > > > > > > > > > > > > > We caught an exceptional address changing while this problem > > > > > happened, show as > > > > > follow: > > > > > > > > > > Before pci_bridge_update_mappings： > > > > > > > > > > fc00-fc1f (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fc00-fc1f > > > > > > > > > > fc20-fc3f (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fc20-fc3f > > > > > > > > > > fc40-fc5f (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fc40-fc5f > > > > > > > > > > fc60-fc7f (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fc60-fc7f > > > > > > > > > > fc80-fc9f (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fc80-fc9f > > > > > <- correct Adress Spce > > > > > > > > > > fca0-fcbf (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fca0-fcbf > > > > > > > > > > fcc0-fcdf (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fcc0-fcdf > > > > > > > > > > fce0-fcff (prio 1, RW): alias > > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > > fce0-fcff > > > > > > > > > > > > > > > > > > > > After pci_bridge_update_mappings： > > > > > > > > > > fda0-fdbf (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fda0-fdbf > > > > > > > > > > fdc0-fddf (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fdc0-fddf > > > > > > > > > > fde0-fdff (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fde0-fdff > > > > > > > > > > fe00-fe1f (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fe00-fe1f > > > > > > > > > > fe20-fe3f (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fe20-fe3f > > > > > > > > > > fe40-fe5f (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fe40-fe5f > > > > > > > > > > fe60-fe7f (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fe60-fe7f > > > > > > > > > > fe80-fe9f (prio 1, RW): alias > > > > > pci_bridge_mem @pci_bridge_pci fe80-fe9f > > > > > > > > > > fc80-fc80 (prio 1, RW): alias > > pci_bridge_pref_mem > > > > > @pci_bridge_pci fc80-fc80 <- Exceptional > > > > > Adress > > > > Space > > > > > > > > This one is empty though right? > > > > > > > > > > > > > > > > > > > We have figured out why this address becomes this value, > > > > > according to pci spec, pci driver can get BAR address size by > > > > > writing 0x to > > > > > > > > > > the pci register firstly, and then read back the value from this > > > > > register. > > > > > > > > > > > > OK however as you show below the BAR being sized is the BAR if a > > > > bridge. Are you then adding a
Re: [Qemu-devel] [PATCH v7 15/19] spapr/xive: enable XIVE MMIOs at reset
On Sun, Dec 09, 2018 at 08:46:06PM +0100, Cédric Le Goater wrote:
> Depending on the interrupt mode chosen, enable or disable the XIVE
> MMIOs.
>
> Signed-off-by: Cédric Le Goater
> ---
> include/hw/ppc/spapr_xive.h | 1 +
> hw/intc/spapr_xive.c| 9 +
> hw/ppc/spapr_irq.c | 8
> 3 files changed, 18 insertions(+)
>
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 7244a6231ce6..308afb61a666 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -48,5 +48,6 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
> void spapr_xive_reset_tctx(sPAPRXive *xive);
> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable);
>
> #endif /* PPC_SPAPR_XIVE_H */
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 560d8d031f74..c6dbb2e8cfc7 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -179,6 +179,15 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
> sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
> }
>
> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable)
The logic looks fine, but I dislike this name - it's called
..._enable() when it can also be used to disable.
> +{
> +memory_region_set_enabled(>source.esb_mmio, enable);
> +memory_region_set_enabled(>tm_mmio, enable);
> +
> +/* Disable the END ESBs until a guest OS makes use of them */
> +memory_region_set_enabled(>end_source.esb_mmio, false);
> +}
> +
> /*
> * When a Virtual Processor is scheduled to run on a HW thread, the
> * hypervisor pushes its identifier in the OS CAM line. Emulate the
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index b423cee30e2c..a8e50725397c 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -217,6 +217,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState
> *spapr, Error **errp)
> CPU_FOREACH(cs) {
> spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> }
> +
> +/* Deactivate the XIVE MMIOs */
> +if (spapr->xive) {
> +spapr_xive_enable_mmio(spapr->xive, false);
> +}
> }
>
> #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
> @@ -358,6 +363,9 @@ static void spapr_irq_reset_xive(sPAPRMachineState
> *spapr, Error **errp)
> * to come after the XiveTCTX reset handlers.
> */
> spapr_xive_reset_tctx(spapr->xive);
> +
> +/* Activate the XIVE MMIOs */
> +spapr_xive_enable_mmio(spapr->xive, true);
> }
>
> /*
--
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: [Qemu-devel] [PATCH v7 18/19] spapr: add a 'pseries-4.0-xive' machine type
On Mon, Dec 10, 2018 at 11:17:33PM +0100, Cédric Le Goater wrote:
> On 12/9/18 8:46 PM, Cédric Le Goater wrote:
> > This pseries machine makes use of a new sPAPR IRQ backend supporting
> > the XIVE interrupt mode.
> >
> > The guest OS is required to have support for the XIVE exploitation
> > mode of the POWER9 interrupt controller.
> >
> > Signed-off-by: Cédric Le Goater
> > ---
> > hw/ppc/spapr.c | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 4012ebd794a4..3cc134a0b673 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3985,6 +3985,21 @@ static void
> > spapr_machine_4_0_class_options(MachineClass *mc)
> >
> > DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> >
> > +static void spapr_machine_4_0_xive_instance_options(MachineState *machine)
> > +{
> > +spapr_machine_4_0_instance_options(machine);
> > +}
> > +
> > +static void spapr_machine_4_0_xive_class_options(MachineClass *mc)
> > +{
> > +sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> > +
> > +spapr_machine_4_0_class_options(mc);> +smc->irq = _irq_xive;
>
> I have been adding checks on the CPU model to export the XIVE capability
> only on POWER9 processors but it breaks some of the tests.
>
> I was wondering if we could add a default POWER9 CPU to the -xive machine :
>
> + mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
>
> and if we could change tests/cpu-plug-test.c with :
>
> @@ -198,8 +198,13 @@ static void add_pseries_test_case(const
>}
>data = g_new(PlugTestData, 1);
>data->machine = g_strdup(mname);
> -data->cpu_model = "power8_v2.0";
> -data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
> +if (g_str_has_suffix(mname, "xive")) {
> +data->cpu_model = "power9_v2.0";
> +data->device_model = g_strdup("power9_v2.0-spapr-cpu-core");
> +} else {
> +data->cpu_model = "power8_v2.0";
> +data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
> +}
>data->sockets = 2;
>data->cores = 3;
>data->threads = 1;
>
> or if there is a better way ?
So, I'd actually prefer a machine option, rather than wholly separate
machine types to select xics/xive/dual. Machine types was fine while
prototyping this, but I don't think we want to actually merge new
machine types for it.
So, instead I think we want a machine option which can be set to
xics/xive/dual, with xics being the default for earlier machine types
and dual the default for 4.0 onwards. We can make POWER9 the default
cpu for 4.0 onwards as well, if you want.
--
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: [Qemu-devel] [PATCH v7 16/19] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS
On Sun, Dec 09, 2018 at 08:46:07PM +0100, Cédric Le Goater wrote:
> The interrupt mode is chosen by the CAS negotiation process and
> activated after a reset to take into account the required changes in
> the machine. These impact the device tree layout, the interrupt
> presenter object and the exposed MMIO regions in the case of XIVE.
>
> This default interrupt mode for the machine is XICS.
>
> Signed-off-by: Cédric Le Goater
> ---
> include/hw/ppc/spapr_irq.h | 1 +
> hw/ppc/spapr.c | 3 +-
> hw/ppc/spapr_hcall.c | 13
> hw/ppc/spapr_irq.c | 143 +
> 4 files changed, 159 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index b34d5a00381b..29936498dbc8 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -51,6 +51,7 @@ typedef struct sPAPRIrq {
> extern sPAPRIrq spapr_irq_xics;
> extern sPAPRIrq spapr_irq_xics_legacy;
> extern sPAPRIrq spapr_irq_xive;
> +extern sPAPRIrq spapr_irq_dual;
>
> void spapr_irq_init(sPAPRMachineState *spapr, Error **errp);
> int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error
> **errp);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5ef87a00f68b..fa41927d95dd 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2631,7 +2631,8 @@ static void spapr_machine_init(MachineState *machine)
> spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>
> /* advertise XIVE */
> -if (smc->irq->ov5 == SPAPR_OV5_XIVE_EXPLOIT) {
> +if (smc->irq->ov5 == SPAPR_OV5_XIVE_EXPLOIT ||
> +smc->irq->ov5 == SPAPR_OV5_XIVE_BOTH) {
> spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
> }
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index ae913d070f50..186b6a65543f 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1654,6 +1654,19 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
> (spapr_h_cas_compose_response(spapr, args[1], args[2],
>ov5_updates) != 0);
> }
> +
> +/*
> + * Generate a machine reset when we have an update of the
> + * interrupt mode. Only required on the machine supporting both
> + * mode.
> + */
> +if (!spapr->cas_reboot) {
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> +
> +spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> +&& smc->irq->ov5 == SPAPR_OV5_XIVE_BOTH;
> +}
> +
> spapr_ovec_cleanup(ov5_updates);
>
> if (spapr->cas_reboot) {
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index a8e50725397c..7c34939f774a 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -392,6 +392,149 @@ sPAPRIrq spapr_irq_xive = {
> .reset = spapr_irq_reset_xive,
> };
>
> +/*
> + * Dual XIVE and XICS IRQ backend.
> + *
> + * Both interrupt mode, XIVE and XICS, objects are created but the
> + * machine starts in legacy interrupt mode (XICS). It can be changed
> + * by the CAS negotiation process and, in that case, the new mode is
> + * activated after extra machine reset.
> + */
> +
> +/*
> + * Returns the sPAPR IRQ backend negotiated by CAS. XICS is the
> + * default.
> + */
> +static sPAPRIrq *spapr_irq_current(sPAPRMachineState *spapr)
> +{
> +return spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT) ?
> +_irq_xive : _irq_xics;
> +}
> +
> +static void spapr_irq_init_dual(sPAPRMachineState *spapr, Error **errp)
> +{
> +MachineState *machine = MACHINE(spapr);
> +Error *local_err = NULL;
> +
> +if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> +error_setg(errp, "No KVM support for the 'dual' machine");
> +return;
> +}
> +
> +spapr_irq_xics.init(spapr, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
> +spapr_irq_xive.init(spapr, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +}
> +
> +static int spapr_irq_claim_dual(sPAPRMachineState *spapr, int irq, bool lsi,
> +Error **errp)
> +{
> +int ret;
> +Error *local_err = NULL;
> +
> +ret = spapr_irq_xive.claim(spapr, irq, lsi, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return ret;
> +}
> +
> +ret = spapr_irq_xics.claim(spapr, irq, lsi, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +}
> +
> +return ret;
> +}
> +
> +static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
> +{
> +spapr_irq_xive.free(spapr, irq, num);
> +spapr_irq_xics.free(spapr, irq, num);
> +}
> +
> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
> +{
> +return spapr_irq_current(spapr)->qirq(spapr, irq);
Urgh... I don't think this is going to work.
Re: [Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug
On Sat, Dec 08, 2018 at 11:58:59AM +, xuyandong wrote: > > > > Hi all, > > > > > > > > > > > > > > > > In our test, we configured VM with several pci-bridges and a > > > > virtio-net nic been attached with bus 4, > > > > > > > > After VM is startup, We ping this nic from host to judge if it is > > > > working normally. Then, we hot add pci devices to this VM with bus 0. > > > > > > > > We found the virtio-net NIC in bus 4 is not working (can not > > > > connect) occasionally, as it kick virtio backend failure with error > > > > below: > > > > > > > > Unassigned mem write fc803004 = 0x1 > > > > > > > > > > > > > > > > memory-region: pci_bridge_pci > > > > > > > > - (prio 0, RW): pci_bridge_pci > > > > > > > > fc80-fc803fff (prio 1, RW): virtio-pci > > > > > > > > fc80-fc800fff (prio 0, RW): > > > > virtio-pci-common > > > > > > > > fc801000-fc801fff (prio 0, RW): > > > > virtio-pci-isr > > > > > > > > fc802000-fc802fff (prio 0, RW): > > > > virtio-pci-device > > > > > > > > fc803000-fc803fff (prio 0, RW): > > > > virtio-pci-notify <- io mem unassigned > > > > > > > > … > > > > > > > > > > > > > > > > We caught an exceptional address changing while this problem > > > > happened, show as > > > > follow: > > > > > > > > Before pci_bridge_update_mappings： > > > > > > > > fc00-fc1f (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fc00-fc1f > > > > > > > > fc20-fc3f (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fc20-fc3f > > > > > > > > fc40-fc5f (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fc40-fc5f > > > > > > > > fc60-fc7f (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fc60-fc7f > > > > > > > > fc80-fc9f (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fc80-fc9f > > > > <- correct Adress Spce > > > > > > > > fca0-fcbf (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fca0-fcbf > > > > > > > > fcc0-fcdf (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fcc0-fcdf > > > > > > > > fce0-fcff (prio 1, RW): alias > > > > pci_bridge_pref_mem @pci_bridge_pci > > > > fce0-fcff > > > > > > > > > > > > > > > > After pci_bridge_update_mappings： > > > > > > > > fda0-fdbf (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fda0-fdbf > > > > > > > > fdc0-fddf (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fdc0-fddf > > > > > > > > fde0-fdff (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fde0-fdff > > > > > > > > fe00-fe1f (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fe00-fe1f > > > > > > > > fe20-fe3f (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fe20-fe3f > > > > > > > > fe40-fe5f (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fe40-fe5f > > > > > > > > fe60-fe7f (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fe60-fe7f > > > > > > > > fe80-fe9f (prio 1, RW): alias > > > > pci_bridge_mem @pci_bridge_pci fe80-fe9f > > > > > > > > fc80-fc80 (prio 1, RW): alias > pci_bridge_pref_mem > > > > @pci_bridge_pci fc80-fc80 <- Exceptional > > > > Adress > > > Space > > > > > > This one is empty though right? > > > > > > > > > > > > > > > We have figured out why this address becomes this value, > > > > according to pci spec, pci driver can get BAR address size by > > > > writing 0x to > > > > > > > > the pci register firstly, and then read back the value from this > > > > register. > > > > > > > > > OK however as you show below the BAR being sized is the BAR if a > > > bridge. Are you then adding a bridge device by hotplug? > > > > No, I just simply hot plugged a VFIO device to Bus 0, another > > interesting phenomenon is If I hot plug the device to other bus, this > > doesn't > happened. > > > > > > > > > > > > We didn't handle this value specially while process pci write in > > > > qemu, the function
Re: [Qemu-devel] [PATCH v7 03/19] ppc/xive: introduce a simplified XIVE presenter
On Mon, Dec 10, 2018 at 08:15:40AM +0100, Cédric Le Goater wrote: > On 12/10/18 5:27 AM, David Gibson wrote: > > On Sun, Dec 09, 2018 at 08:45:54PM +0100, Cédric Le Goater wrote: > >> The last sub-engine of the XIVE architecture is the Interrupt > >> Virtualization Presentation Engine (IVPE). On HW, the IVRE and the > >> IVPE share elements, the Power Bus interface (CQ), the routing table > >> descriptors, and they can be combined in the same HW logic. We do the > >> same in QEMU and combine both engines in the XiveRouter for > >> simplicity. > >> > >> When the IVRE has completed its job of matching an event source with a > >> Notification Virtual Target (NVT) to notify, it forwards the event > >> notification to the IVPE sub-engine. The IVPE scans the thread > >> interrupt contexts of the Notification Virtual Targets (NVT) > >> dispatched on the HW processor threads and if a match is found, it > >> signals the thread. If not, the IVPE escalates the notification to > >> some other targets and records the notification in a backlog queue. > >> > >> The IVPE maintains the thread interrupt context state for each of its > >> NVTs not dispatched on HW processor threads in the Notification > >> Virtual Target table (NVTT). > >> > >> The model currently only supports single NVT notifications. > >> > >> Signed-off-by: Cédric Le Goater > > > > Applied. > > > > I think the tctx_word2() should have the byteswap, rather than having > > it in the callers, but that can be fixed later. > > I thought it was better to explicitly show in the code where the > byteswaps were needed. Anyway, this is very localized, so, yes, > we can change it later on. To clarify my thinking here; the important thing is not knowing where the byteswaps are, but being able to tell as easily as possible what endianness a given piece of data is right now. The convention I'm aiming for here - which is one I try to use most places is that structures - at least structures which map to specific in-memory things - are in the required endianness for that stuff in memory. However bare integers - uint32_t or uint64_t or whatever - are, well, numbers, in native endian. -- 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: [Qemu-devel] [RFC PATCH v2 3/3] target/ppc: support single stepping with KVM HV
On Mon, Dec 10, 2018 at 10:52:18AM -0200, Fabiano Rosas wrote:
> David Gibson writes:
>
> >> >> +if (arch_info->address == trace_handler_addr) {
> >> >> +cpu_synchronize_state(cs);
> >> >> +kvm_remove_breakpoint(cs, trace_handler_addr, 4,
> >> >> GDB_BREAKPOINT_SW);
> >> >> +
> >> >> +cpu_memory_rw_debug(cs, env->spr[SPR_SRR0] - 4, (uint8_t
> >> >> *),
> >> >> +sizeof(insn), 0);
> >> >> +
> >> >> +/* If the last instruction was a mfmsr, make sure that the
> >> >> + * MSR_SE bit is not set to avoid the guest kernel knowing
> >> >> + * that it is being single-stepped */
> >> >> +if (extract32(insn, 26, 6) == 31 && extract32(insn, 1, 10) ==
> >> >> 83) {
> >> >> +reg = extract32(insn, 21, 5);
> >> >> +env->gpr[reg] &= ~(1ULL << MSR_SE);
> >> >> +}
> >> >
> >> > Hm. What happens if both qemu and the guest itself attempt to single
> >> > step at the same time? How do you distinguish between the cases?
> >>
> >> There is currently no distinction being made.
> >
> > This seems incorrect to me. Basically you're restoring !MSR_SE
> > unconditionaly when you finish the hypervisor side step, which might
> > not be correct if the guest is also attempting to single step itself.
> > AFAICT it should be possible to track what the guest thinks is the
> > value of MSR_SE and restore that.
>
> I was skeptical of being able to do both single steps at the same time
> but I found a way to reproduce it by stepping over an rfid when
> SRR1_SE is already 1.
>
> > If both hypervisor and guest
> > attempt to single step, I'd expect to have the hypervisor trap first,
> > then return to the guest's single step vector.
>
> With the fix you suggest above, QEMU will be able to single step the
> interrupt handler during the guest's single step. That means I'll have
> to restore the SRRs as well so that the handler returns to the correct
> place.
>
> >> I could do the latter, if you prefer.
> >
> > I think that's better - I don't think it's safe to assume that you're
> > *not* synchronized with KVM.
>
> Ok, that's better indeed.
>
> Thanks for the comments, I'll prepare another version with the
> appropriate corrections.
Ok, great.
--
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: [Qemu-devel] [Qemu-ppc] [RFC PATCH 0/6] target/ppc: convert VMX instructions to use TCG vector operations
On Mon, Dec 10, 2018 at 09:54:51PM +0100, BALATON Zoltan wrote: > On Mon, 10 Dec 2018, David Gibson wrote: > > On Mon, Dec 10, 2018 at 01:33:53AM +0100, BALATON Zoltan wrote: > > > On Fri, 7 Dec 2018, Mark Cave-Ayland wrote: > > > > This patchset is an attempt at trying to improve the VMX (Altivec) > > > > instruction > > > > performance by making use of the new TCG vector operations where > > > > possible. > > > > > > This is very welcome, thanks for doing this. > > > > > > > In order to use TCG vector operations, the registers must be accessible > > > > from cpu_env > > > > whilst currently they are accessed via arrays of static TCG globals. > > > > Patches 1-3 > > > > are therefore mechanical patches which introduce access helpers for > > > > FPR, AVR and VSR > > > > registers using the supplied TCGv_i64 parameter. > > > > > > Have you tried some benchmarks or tests to measure the impact of these > > > changes? I've tried the (very unscientific) benchmarks I've written about > > > before here: > > > > > > http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00261.html > > > > > > (which seem to use AltiVec/VMX instructions but not sure which) on mac99 > > > with MorphOS and I could not see any performance increase. I haven't run > > > enough tests but results with or without this series on master were mostly > > > the same within a few percents, and sometimes even seen lower performance > > > with these patches than without. I haven't tried to find out why (no time > > > for that now) so can't really draw any conclusions from this. I'm also not > > > sure if I've actually tested what you've changed or these use instructions > > > that your patches don't optimise yet, or the changes I've seen were just > > > normal changes between runs; but I wonder if the increased number of > > > temporaries could result in lower performance in some cases? > > > > What was your host machine. IIUC this change will only improve > > performance if the host tcg backend is able to implement TCG vector > > ops in terms of vector ops on the host. > > Tried it on i5 650 which has: sse sse2 ssse3 sse4_1 sse4_2. I assume x86_64 > should be supported but not sure what are the CPU requirements. > > > In addition, this series only converts a subset of the integer and > > logical vector instructions. If your testcase is mostly floating > > point (vectored or otherwise), it will still be softfloat and so not > > see any speedup. > > Yes, I don't really know what these tests use but I think "lame" test is > mostly floating point but tried with "lame_vmx" which should at least use > some vector ops and "mplayer -benchmark" test is more vmx dependent based on > my previous profiling and testing with hardfloat but I'm not sure. (When > testing these with hardfloat I've found that lame was benefiting from > hardfloat but mplayer wasn't and more VMX related functions showed up with > mplayer so I assumed it's more VMX bound.) I should clarify here. When I say "floating point" above, I'm not meaning things using the regular FPU instead of the vector unit. I'm saying *anything* involving floating point calculations whether they're done in the FPU or the vector unit. The patches here don't convert all VMX instructions to use vector TCG ops - they only convert a few, and those few are about using the vector unit for integer (and logical) operations. VMX instructions involving floating point calculations are unaffected and will still use soft-float. > I've tried to do some profiling again to find out what's used but I can't > get good results with the tools I have (oprofile stopped working since I've > updated my machine and Linux perf provides results that are hard to > interpret for me, haven't tried if gprof would work now it didn't before) > but I've seen some vector related helpers in the profile so at least some > vector ops are used. The "helper_vperm" came up top at about 11th (not sure > where is it called from), other vector helpers were lower. > > I don't remember details now but previously when testing hardfloat I've > written this: "I've looked at vperm which came out top in one of the > profiles I've taken and on little endian hosts it has the loop backwards and > also accesses vector elements from end to front which I wonder may be enough > for the compiler to not be able to optimise it? But I haven't checked > assembly. The altivec dependent mplayer video decoding test did not change > much with hardfloat, it took 98% compared to master so likely altivec is > dominating here." (Although this was with the PPC specific vector helpers > before VMX patch so not sure if this is still relevant.) > > The top 10 in profile were still related to low level memory access and MMU > management stuff as I've found before: > > http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg03609.html > http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg03704.html > > I think implementing i2c for mac99 may help
[Qemu-devel] [PATCH v2] usb-host: reset and close libusb_device_handle before qemu exit
we should perform these actions as same as usb_host_close.
Signed-off-by: linzhecheng
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index b6602ded4e..833250a886 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -988,7 +988,9 @@ static void usb_host_exit_notifier(struct Notifier *n, void
*data)
if (s->dh) {
usb_host_release_interfaces(s);
+libusb_reset_device(s->dh);
usb_host_attach_kernel(s);
+libusb_close(s->dh);
}
}
--
2.12.2.windows.2
Re: [Qemu-devel] [PATCH v7 09/19] spapr: add device tree support for the XIVE exploitation mode
On Mon, Dec 10, 2018 at 08:53:17AM +0100, Cédric Le Goater wrote:
> On 12/10/18 7:39 AM, David Gibson wrote:
> > On Sun, Dec 09, 2018 at 08:46:00PM +0100, Cédric Le Goater wrote:
> >> The XIVE interface for the guest is described in the device tree under
> >> the "interrupt-controller" node. A couple of new properties are
> >> specific to XIVE :
> >>
> >> - "reg"
> >>
> >>contains the base address and size of the thread interrupt
> >>managnement areas (TIMA), for the User level and for the Guest OS
> >>level. Only the Guest OS level is taken into account today.
> >>
> >> - "ibm,xive-eq-sizes"
> >>
> >>the size of the event queues. One cell per size supported, contains
> >>log2 of size, in ascending order.
> >>
> >> - "ibm,xive-lisn-ranges"
> >>
> >>the IRQ interrupt number ranges assigned to the guest for the IPIs.
> >>
> >> and also under the root node :
> >>
> >> - "ibm,plat-res-int-priorities"
> >>
> >>contains a list of priorities that the hypervisor has reserved for
> >>its own use. OPAL uses the priority 7 queue to automatically
> >>escalate interrupts for all other queues (DD2.X POWER9). So only
> >>priorities [0..6] are allowed for the guest.
> >>
> >> Extend the sPAPR IRQ backend with a new handler to populate the DT
> >> with the appropriate "interrupt-controller" node.
> >>
> >> Signed-off-by: Cédric Le Goater
> >> ---
> >> include/hw/ppc/spapr_irq.h | 2 ++
> >> include/hw/ppc/spapr_xive.h | 2 ++
> >> include/hw/ppc/xics.h | 4 +--
> >> hw/intc/spapr_xive.c| 64 +
> >> hw/intc/xics_spapr.c| 3 +-
> >> hw/ppc/spapr.c | 3 +-
> >> hw/ppc/spapr_irq.c | 3 ++
> >> 7 files changed, 77 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> >> index 23cdb51b879e..e51e9f052f63 100644
> >> --- a/include/hw/ppc/spapr_irq.h
> >> +++ b/include/hw/ppc/spapr_irq.h
> >> @@ -39,6 +39,8 @@ typedef struct sPAPRIrq {
> >> void (*free)(sPAPRMachineState *spapr, int irq, int num);
> >> qemu_irq (*qirq)(sPAPRMachineState *spapr, int irq);
> >> void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
> >> +void (*dt_populate)(sPAPRMachineState *spapr, uint32_t nr_servers,
> >> +void *fdt, uint32_t phandle);
> >> } sPAPRIrq;
> >>
> >> extern sPAPRIrq spapr_irq_xics;
> >> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> >> index 9506a8f4d10a..728a5e8dc163 100644
> >> --- a/include/hw/ppc/spapr_xive.h
> >> +++ b/include/hw/ppc/spapr_xive.h
> >> @@ -45,5 +45,7 @@ qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn);
> >> typedef struct sPAPRMachineState sPAPRMachineState;
> >>
> >> void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> >> +void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void
> >> *fdt,
> >> + uint32_t phandle);
> >>
> >> #endif /* PPC_SPAPR_XIVE_H */
> >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> >> index 9958443d1984..14afda198cdb 100644
> >> --- a/include/hw/ppc/xics.h
> >> +++ b/include/hw/ppc/xics.h
> >> @@ -181,8 +181,6 @@ typedef struct XICSFabricClass {
> >> ICPState *(*icp_get)(XICSFabric *xi, int server);
> >> } XICSFabricClass;
> >>
> >> -void spapr_dt_xics(int nr_servers, void *fdt, uint32_t phandle);
> >> -
> >> ICPState *xics_icp_get(XICSFabric *xi, int server);
> >>
> >> /* Internal XICS interfaces */
> >> @@ -204,6 +202,8 @@ void icp_resend(ICPState *ss);
> >>
> >> typedef struct sPAPRMachineState sPAPRMachineState;
> >>
> >> +void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void
> >> *fdt,
> >> + uint32_t phandle);
> >> int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
> >> void xics_spapr_init(sPAPRMachineState *spapr);
> >>
> >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> >> index 982ac6e17051..a6d854b07690 100644
> >> --- a/hw/intc/spapr_xive.c
> >> +++ b/hw/intc/spapr_xive.c
> >> @@ -14,6 +14,7 @@
> >> #include "target/ppc/cpu.h"
> >> #include "sysemu/cpus.h"
> >> #include "monitor/monitor.h"
> >> +#include "hw/ppc/fdt.h"
> >> #include "hw/ppc/spapr.h"
> >> #include "hw/ppc/spapr_xive.h"
> >> #include "hw/ppc/xive.h"
> >> @@ -1381,3 +1382,66 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
> >> spapr_register_hypercall(H_INT_SYNC, h_int_sync);
> >> spapr_register_hypercall(H_INT_RESET, h_int_reset);
> >> }
> >> +
> >> +void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void
> >> *fdt,
> >> + uint32_t phandle)
> >> +{
> >> +sPAPRXive *xive = spapr->xive;
> >> +int node;
> >> +uint64_t timas[2 * 2];
> >> +/* Interrupt number ranges for the IPIs */
> >> +uint32_t lisn_ranges[] = {
> >> +cpu_to_be32(0),
> >> +cpu_to_be32(nr_servers),
> >> +};
> >> +uint32_t eq_sizes[] = {
> >> +
Re: [Qemu-devel] [PATCH] cpus.c: Fix race condition in cpu_stop_current()
On Fri, Dec 07, 2018 at 15:59:11 +, Peter Maydell wrote: > We use cpu_stop_current() to ensure the current CPU has stopped > from places like qemu_system_reset_request(). Unfortunately its > current implementation has a race. It calls qemu_cpu_stop(), > which sets cpu->stopped to true even though the CPU hasn't > actually stopped yet. The main thread will look at the flags > set by qemu_system_reset_request() and call pause_all_vcpus(). > pause_all_vcpus() waits for every cpu to have cpu->stopped true, > so it can continue (and we will start the system reset operation) > before the vcpu thread has got back to its top level loop. > > Instead, just set cpu->stop and call cpu_exit(). This will > cause the vcpu to exit back to the top level loop, and there > (as part of the wait_io_event code) it will call qemu_cpu_stop(). > > This fixes bugs where the reset request appeared to be ignored > or the CPU misbehaved because the reset operation started > to change vcpu state while the vcpu thread was still using it. > > Signed-off-by: Peter Maydell Reviewed-by: Emilio G. Cota Thanks, E.
Re: [Qemu-devel] [RFC PATCH 1/6] target/ppc: introduce get_fpr() and set_fpr() helpers for FP register access
On Mon, Dec 10, 2018 at 12:25:26PM -0600, Richard Henderson wrote: > On 12/9/18 11:17 PM, David Gibson wrote: > > On Fri, Dec 07, 2018 at 08:56:30AM +, Mark Cave-Ayland wrote: > >> These helpers allow us to move FP register values to/from the specified > >> TCGv_i64 > >> argument. > >> > >> To prevent FP helpers accessing the cpu_fpr array directly, add extra TCG > >> temporaries as required. > > > > It's not obvious to me why that's a desirable thing. I'm assuming > > it's somehow necessary for the stuff later in the series, but I think > > we need a brief rationale here to explain why this isn't just adding > > extra reg copies for the sake of it. > > Note that while this introduces extra opcodes, in many cases it does not > change > the number of machine instructions that are generated. Recall that accessing > cpu_fpr[N] implies a load from env. This change makes the load explicit. I realised that a bit later in looking at the series. I think a paraphrasing of the above in the commit message would still be helpful. > The change does currently prevent caching cpu_fpr[N] in a host register. That > can and will be fixed by optimizing on memory operations instead. (There is a > patch that has been outstanding for 13 months to do this. I intend to finally > get around to merging it during the 4.0 cycle.) > > > r~ > -- 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: [Qemu-devel] [RFC PATCH 07/13] tests/tcg/xtensa: enable system tests
On Mon, Dec 10, 2018 at 7:28 AM Alex Bennée wrote: > > Signed-off-by: Alex Bennée > --- > tests/tcg/xtensa/Makefile| 93 > tests/tcg/xtensa/Makefile.softmmu-target | 43 +++ > 2 files changed, 43 insertions(+), 93 deletions(-) > delete mode 100644 tests/tcg/xtensa/Makefile That Makefile provides a few nice goals for guest and host debugging and a way to run tests on Tensilica ISS, it would be nice to keep it. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH for-3.2 01/11] vhost-user: define conventions for vhost-user backends
On Mon, Dec 10, 2018 at 10:36:29PM +0400, Marc-André Lureau wrote: > Hi > > On Mon, Dec 10, 2018 at 6:30 PM Gerd Hoffmann wrote: > > > > On Mon, Nov 26, 2018 at 04:42:40PM +0400, Marc-André Lureau wrote: > > > As discussed during "[PATCH v4 00/29] vhost-user for input & GPU" > > > review, let's define a common set of backend conventions to help with > > > management layer implementation, and interoperability. > > > > > > Signed-off-by: Marc-André Lureau > > > Reviewed-by: Daniel P. Berrangé > > > > Acked-by: Gerd Hoffmann > > > > btw: have you seen the idea to use a vfio-style interface for > > communication between qemu and external device emulation processes? > > I heard there was some discussion during KVM forum. > > Fwiwi, this is also an idea I proposed last year (and quickly > discussed during my talk about multi-process qemu). > I also experimented with the idea, and wrote a vfio-user backend, with > a PCI serial device running in a seperate process: > the qemu tree: https://github.com/elmarco/qemu/tree/wip/vfio-user (dirty tree) > and the serial device: > https://github.com/elmarco/qemu/blob/wip/vfio-user/contrib/libvfio-user/vfio-user-serial.c Right. The main issue is that we need to make sure only in-tree devices are supported. vhost-user by design is for out of tree users. It needn't be hard, maybe it's enough to just make qemu launch these processes as opposed to connecting to them on command line. > > > > > > cheers, > > Gerd > >
Re: [Qemu-devel] [PATCH 2/3] virtio_net: Add support for "Data Path Switching" during Live Migration.
On Mon, Dec 10, 2018 at 12:22:37PM -0800, si-wei liu wrote:
>
>
> On 12/10/2018 9:41 AM, Michael S. Tsirkin wrote:
> > On Mon, Dec 10, 2018 at 11:15:47AM -0500, Venu Busireddy wrote:
> > > Added two new events, FAILOVER_PLUG_PRIMARY and FAILOVER_UNPLUG_PRIMARY.
> > > The first is emitted when the guest negotiates the F_STANDBY feature
> > > bit. The second is emitted when the virtio_net driver is removed, either
> > > manually or as a result of guest reboot.
> > So the names mean "should plug" and "should unplug" right?
> Sounds about right, but management stack may ignore the message if not going
> to plug in the primary device.
>
> > It seems inelegant to tell upper layers what they should do.
> > After all they are upper layers because presumably
> > there is a policy question as opposed to a mechanism.
> > Can we expose information without telling managmenent what to do?
> Is it just the name itself that is offensive? The information exposed to
> upper layer is just that time is ready to plug/unplug the paired device.
> Would the names FAILOVER_STANDY_SET and FAILOVER_STANDY_CLEAR fit your
> expectation?
So really the same thing applies as with the other events:
event should just signal a change, status should be
reported with a query command. Avoids event floods
and handles management restarts gracefully.
> > Alternatively, is there a real need to unplug the device completely?
> > Would it work to just hide the device from guest instead? QEMU can do
> > it itself and this is the direction that Sameeh has been taking.
> See below in the cover letter:
>
> 3. While the hidden device model (viz. coupled device model) is still
> being explored for automatic hot plugging (QEMU) and automatic datapath
> switching (host-kernel), this series provides a supplemental set
> of interfaces if management software wants to drive the SR-IOV live
> migration on its own. It should not conflict with the hidden device
> model but just offers simplicity of implementation.
>
> As said this is a supplemental implementation that involves and leverages
> management software before we can converge to the ideal hidden/coupled
> device model.
So I don't think there's a problem with sending an event when
the feature bit gets set/cleared, as long as there's
no implication that management is required to react
in a certain way. So yes, it's a naming issue at that level.
> As I understand it, we're still exploring the best way to handle the
> datapath (MAC filter) switching problem for the hidden (coupled) device
> model, right. Even if we can hide the device there's still need to inform
> upper layer for datapath switching that is currently being handled in
> userspace management stack. I think the best fit for the hidden (coupled)
> model is that all datapath switching can be handled automatically and
> transparently in the kernel without needing to notify userspace and depend
> on userspace reaction.
>
> >
> > > Management stack can use these
> > > events to determine when to plug/unplug the VF device to/from the guest.
> > >
> > > Also, the Virtual Functions will be automatically removed from the guest
> > > if the guest is rebooted. To properly identify the VFIO devices that
> > > must be removed, a new property named "x-failover-primary" is added to
> > > the vfio-pci devices. Only the vfio-pci devices that have this property
> > > enabled are removed from the guest upon reboot.
> > If this property needs to be set by management then
> > it must not start with "x-" - that prefix means
> > "unstable do not use from external tools".
>
> Sure, will remove "x-" (sorry, missed to correct it before sending out
> publicly).
>
> Thanks,
> -Siwei
>
> >
> > > Signed-off-by: Venu Busireddy
> >
> >
> > > ---
> > > hw/acpi/pcihp.c| 27 ++
> > > hw/net/virtio-net.c| 23 ++
> > > hw/vfio/pci.c | 3 +++
> > > hw/vfio/pci.h | 1 +
> > > include/hw/pci/pci.h | 1 +
> > > include/hw/virtio/virtio-net.h | 4
> > > qapi/net.json | 44
> > > ++
> > > 7 files changed, 103 insertions(+)
> > >
> > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > > index 80d42e1..2a3ffd3 100644
> > > --- a/hw/acpi/pcihp.c
> > > +++ b/hw/acpi/pcihp.c
> > > @@ -176,6 +176,25 @@ static void acpi_pcihp_eject_slot(AcpiPciHpState *s,
> > > unsigned bsel, unsigned slo
> > > }
> > > }
> > > +static void acpi_pcihp_cleanup_failover_primary(AcpiPciHpState *s, int
> > > bsel)
> > > +{
> > > +BusChild *kid, *next;
> > > +PCIBus *bus = acpi_pcihp_find_hotplug_bus(s, bsel);
> > > +
> > > +if (!bus) {
> > > +return;
> > > +}
> > > +QTAILQ_FOREACH_SAFE(kid, >qbus.children, sibling, next) {
> > > +DeviceState *qdev = kid->child;
> > > +PCIDevice *pdev = PCI_DEVICE(qdev);
> > > +int slot =
Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 0/6] target/ppc: convert VMX instructions to use TCG vector operations
On Mon, 10 Dec 2018, Richard Henderson wrote: On 12/10/18 2:54 PM, BALATON Zoltan wrote: Tried it on i5 650 which has: sse sse2 ssse3 sse4_1 sse4_2. I assume x86_64 should be supported but not sure what are the CPU requirements. Not quite. I only support avx1 and later. I thought about supporting sse4 and later (that's the minimum with all of the instructions that do what we need), but there is only one cpu generation with sse4 and without avx1, and avx1 is already 8 years old. OK that explains why I haven't seen any improvements. My CPU just predates AVX and happens to be in the generation you mention. But I agree this probably does not worth the effort. Maybe I should test on something newer instead. Thank you, BALATON Zoltan
Re: [Qemu-devel] [PATCH 04/14] qemu-nbd: Simplify --partition handling
On 11/30/18 4:03 PM, Eric Blake wrote:
Our open-coding of strtol handling forgot to handle overflow
conditions. What's more, since we insiste on a user-supplied
partition to be non-zero, we can use 0 rather than -1 for our
initial value to distinguish when a partition is not being
served, for slightly more optimal code.
Signed-off-by: Eric Blake
---
qemu-nbd.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 55e29bd9a7e..866e64779f1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -546,7 +546,7 @@ int main(int argc, char **argv)
int opt_ind = 0;
char *end;
int flags = BDRV_O_RDWR;
-int partition = -1;
+int partition = 0;
int ret = 0;
bool seen_cache = false;
bool seen_discard = false;
@@ -685,13 +685,9 @@ int main(int argc, char **argv)
flags &= ~BDRV_O_RDWR;
break;
case 'P':
-partition = strtol(optarg, , 0);
-if (*end) {
-error_report("Invalid partition `%s'", optarg);
-exit(EXIT_FAILURE);
-}
-if (partition < 1 || partition > 8) {
-error_report("Invalid partition %d", partition);
+if (qemu_strtoi(optarg, NULL, 0, ) < 0 ||
Hmm - the fact that 'char *end' is not a dead variable means there are
more uses of strtoll() that need fixing. I'll get those in v2.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v7 18/19] spapr: add a 'pseries-4.0-xive' machine type
On 12/9/18 8:46 PM, Cédric Le Goater wrote:
> This pseries machine makes use of a new sPAPR IRQ backend supporting
> the XIVE interrupt mode.
>
> The guest OS is required to have support for the XIVE exploitation
> mode of the POWER9 interrupt controller.
>
> Signed-off-by: Cédric Le Goater
> ---
> hw/ppc/spapr.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4012ebd794a4..3cc134a0b673 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3985,6 +3985,21 @@ static void
> spapr_machine_4_0_class_options(MachineClass *mc)
>
> DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
>
> +static void spapr_machine_4_0_xive_instance_options(MachineState *machine)
> +{
> +spapr_machine_4_0_instance_options(machine);
> +}
> +
> +static void spapr_machine_4_0_xive_class_options(MachineClass *mc)
> +{
> +sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
> +spapr_machine_4_0_class_options(mc);> +smc->irq = _irq_xive;
I have been adding checks on the CPU model to export the XIVE capability
only on POWER9 processors but it breaks some of the tests.
I was wondering if we could add a default POWER9 CPU to the -xive machine :
+ mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
and if we could change tests/cpu-plug-test.c with :
@@ -198,8 +198,13 @@ static void add_pseries_test_case(const
}
data = g_new(PlugTestData, 1);
data->machine = g_strdup(mname);
-data->cpu_model = "power8_v2.0";
-data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
+if (g_str_has_suffix(mname, "xive")) {
+data->cpu_model = "power9_v2.0";
+data->device_model = g_strdup("power9_v2.0-spapr-cpu-core");
+} else {
+data->cpu_model = "power8_v2.0";
+data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
+}
data->sockets = 2;
data->cores = 3;
data->threads = 1;
or if there is a better way ?
Thanks,
C.
> +}
> +
> +DEFINE_SPAPR_MACHINE(4_0_xive, "4.0-xive", false);
> +
> /*
> * pseries-3.1
> */
>
Re: [Qemu-devel] [PATCH] target/arm: Implement VI/VF bits
On Mon, 10 Dec 2018 at 21:32, Wedson Almeida Filho wrote: > > Hi Peter, > > We put this together months ago and hadn't kept up to date with the changes > here. > > We're happy to see that support has been added for the VI bit. Is it going to > be available in the v3.1.0 release? Yes, that commit is in 3.1.0 -- do test it and let me know if there's still an issue in this area for your guest code. thanks -- PMM
Re: [Qemu-devel] [PATCH 02/14] nbd/client: More consistent error messages
On 12/5/18 9:03 AM, Vladimir Sementsov-Ogievskiy wrote:
01.12.2018 1:03, Eric Blake wrote:
Consolidate on using decimal (not hex) and on outputting the
option reply name (not just value) when the client reports
protocol discrepancies from the server. While it won't affect
normal operation, it makes debugging additions easier.
Signed-off-by: Eric Blake
---
+error_setg(errp, "Unexpected option type %u (%s) expected %u (%s)",
+ reply->option, nbd_opt_lookup(reply->option),
+ opt, nbd_opt_lookup(opt));
@@ -378,9 +380,9 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
return 1;
}
if (reply.type != NBD_REP_INFO) {
-error_setg(errp, "unexpected reply type %" PRIu32
- " (%s), expected %u",
- reply.type, nbd_rep_lookup(reply.type), NBD_REP_INFO);
+error_setg(errp, "unexpected reply type %u (%s), expected %u (%s)",
hmm, we are definitely inconsistent about having comma before "expected" word...
anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy
That's minor enough; I'll add commas to all instances, but keep your R-b.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [for-4.0 PATCH v4 4/9] qapi: Define PCIe link speed and width properties
On 12/7/18 10:41 AM, Alex Williamson wrote:
Create properties to be able to define speeds and widths for PCIe
links. The only tricky bit here is that our get and set callbacks
translate from the fixed QAPI automagic enums to those we define
in PCI code to represent the actual register segment value.
Cc: Eric Blake
Tested-by: Geoffrey McRae
Reviewed-by: Markus Armbruster
Signed-off-by: Alex Williamson
---
hw/core/qdev-properties.c| 178 ++
include/hw/qdev-properties.h |8 ++
qapi/common.json | 42 ++
3 files changed, 228 insertions(+)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 35072dec1ecf..f5ca5b821a79 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1327,3 +1327,181 @@ const PropertyInfo qdev_prop_off_auto_pcibar = {
.set = set_enum,
.set_default_value = set_default_value_enum,
};
+
+/* --- PCIELinkSpeed 2_5/5/8/16 -- */
+
+static void get_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
+PCIELinkSpeed speed;
C does not guarantee what width 'speed' has,...
+
+switch (*p) {
+case QEMU_PCI_EXP_LNK_2_5GT:
+speed = PCIE_LINK_SPEED_2_5;
+break;
+case QEMU_PCI_EXP_LNK_5GT:
+speed = PCIE_LINK_SPEED_5;
+break;
+case QEMU_PCI_EXP_LNK_8GT:
+speed = PCIE_LINK_SPEED_8;
+break;
+case QEMU_PCI_EXP_LNK_16GT:
+speed = PCIE_LINK_SPEED_16;
+break;
+default:
+/* Unreachable */
+abort();
+}
+
+visit_type_enum(v, prop->name, (int *), prop->info->enum_table,
errp);
...making this cast to (int*) not be very kosher. I _think_ we're okay
here, but I'd be a LOT happier if you just used 'int speed' to avoid the
cast.
+}
+
+static void set_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
+PCIELinkSpeed speed;
+Error *local_err = NULL;
+
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
+visit_type_enum(v, prop->name, (int *),
And again.
+prop->info->enum_table, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+switch (speed) {
+case PCIE_LINK_SPEED_2_5:
+*p = QEMU_PCI_EXP_LNK_2_5GT;
+break;
+case PCIE_LINK_SPEED_5:
+*p = QEMU_PCI_EXP_LNK_5GT;
+break;
+case PCIE_LINK_SPEED_8:
+*p = QEMU_PCI_EXP_LNK_8GT;
+break;
+case PCIE_LINK_SPEED_16:
+*p = QEMU_PCI_EXP_LNK_16GT;
+break;
+default:
+/* Unreachable */
+abort();
+}
+}
+
+static void get_prop_pcielinkwidth(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+PCIExpLinkWidth *p = qdev_get_prop_ptr(dev, prop);
+PCIELinkWidth width;
+
+switch (*p) {
+visit_type_enum(v, prop->name, (int *), prop->info->enum_table,
errp);
+}
+
+static void set_prop_pcielinkwidth(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+PCIExpLinkWidth *p = qdev_get_prop_ptr(dev, prop);
+PCIELinkWidth width;
+Error *local_err = NULL;
+
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
+visit_type_enum(v, prop->name, (int *),
And two more spots.
+++ b/qapi/common.json
@@ -127,6 +127,48 @@
{ 'enum': 'OffAutoPCIBAR',
'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
+##
+# @PCIELinkSpeed:
+#
+# An enumeration of PCIe link speeds in units of GT/s
+#
+# @2_5: 2.5GT/s
+#
+# @5: 5.0GT/s
+#
+# @8: 8.0GT/s
+#
+# @16: 16.0GT/s
+#
+# Since: 4.0
+##
+{ 'enum': 'PCIELinkSpeed',
+ 'data': [ '2_5', '5', '8', '16' ] }
QAPI enums are special-cased to allow starting with digits, thanks to
QKeyCode. I don't know if we are trying to avoid adding yet more of
those, but the fact that you didn't have to whitelist them means that we
are at least not forcibly limiting the use of leading digits in new enums.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH for-4.0 0/5] tcg/i386: Improve guest_base handling
On Mon, Dec 03, 2018 at 10:08:35 -0600, Richard Henderson wrote: > This tidies guest_base handling such that (1) we require no scratch > registers, (2) we require no extra instructions besides the memory op, > and (3) we reduce the size of the memory op by omitting a prefix. > > In principal point 3 is offset by adding additional opcodes to handle > zero-extension when converting 64-bit guest values back to 32-bit guest > addresses. But those turn out to be hen's teeth, since 32-bit guests > often have no way of even producing 64-bit guest values. > > In particular, I saw none in a simple pass through linux-user-test-0.3 > for i386, arm, sh4, sparc. Reviewed-by: Emilio G. Cota for the series. Thanks, Emilio
Re: [Qemu-devel] [PATCH] target/arm: Implement VI/VF bits
Hi Peter, We put this together months ago and hadn't kept up to date with the changes here. We're happy to see that support has been added for the VI bit. Is it going to be available in the v3.1.0 release? Thanks, -Wedson On Mon, Dec 10, 2018 at 8:41 PM Peter Maydell wrote: > On Mon, 10 Dec 2018 at 18:38, Andrew Walbran wrote: > > > > From: Wedson Almeida Filho > > > > This lets hypervisors running at EL2 inject virtual interrupts into > guest VMs. > > > > Signed-off-by: Wedson Almeida Filho > > Signed-off-by: Andrew Walbran > > Hi; thanks for this patch. I'm a bit confused -- can you > explain what the patch provides that isn't already > implemented by commit 89430fc6f80a5aef1d4cb ? > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH 2/8] vfio: make vfio_address_spaces static
On Mon, 10 Dec 2018 19:28:10 +0100 Paolo Bonzini wrote: > It is not used outside hw/vfio/common.c, so it does not need to > be extern. > > Cc: Alex Williamson > Signed-off-by: Paolo Bonzini > --- > hw/vfio/common.c | 2 +- > include/hw/vfio/vfio-common.h | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) Acked-by: Alex Williamson > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 7c185e5a2e..7aa804ea0b 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -39,7 +39,7 @@ > > struct vfio_group_head vfio_group_list = > QLIST_HEAD_INITIALIZER(vfio_group_list); > -struct vfio_as_head vfio_address_spaces = > +static QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces = > QLIST_HEAD_INITIALIZER(vfio_address_spaces); > > #ifdef CONFIG_KVM > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index 1b434d02f6..127ca47815 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -181,7 +181,6 @@ int vfio_get_device(VFIOGroup *group, const char *name, > > extern const MemoryRegionOps vfio_region_ops; > extern QLIST_HEAD(vfio_group_head, VFIOGroup) vfio_group_list; > -extern QLIST_HEAD(vfio_as_head, VFIOAddressSpace) vfio_address_spaces; > > #ifdef CONFIG_LINUX > int vfio_get_region_info(VFIODevice *vbasedev, int index,
Re: [Qemu-devel] [RFC PATCH 7/7] virtio-fs: Allow mapping of journal
On 12/10/18 11:31 AM, Dr. David Alan Gilbert (git) wrote: From: "Dr. David Alan Gilbert" The 'journal' is a shared block of RAM between QEMU and it's s/it's/its/ (here, you want possessive) fuse daemon. It's typically a shmfs file and it's specified as: whereas here, both uses of "it's" are correct as contractions for "it is" (although I might use just "is" instead of "it's" for that last instance). -device vhost-user-fs-pci,chardev=char0,tag=myfs,cache-size=1G,versiontable=/dev/shm/mdvt1,journal=/dev/shm/journal1 It gets mapped into the PCI bar after the version table. Signed-off-by: Dr. David Alan Gilbert --- -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC PATCH 3/7] virtio-fs: Add cache BAR
On 12/10/18 11:31 AM, Dr. David Alan Gilbert (git) wrote: From: "Dr. David Alan Gilbert" Add a cache BAR into which files will be directly mapped. The size cacn be set with the cache-size= property, e.g. s/cacn/can/ -device vhost-user-fs-pci,chardev=char0,tag=myfs,cache-size=16G Signed-off-by: Dr. David Alan Gilbert --- -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 0/6] target/ppc: convert VMX instructions to use TCG vector operations
On 12/10/18 2:54 PM, BALATON Zoltan wrote: >> What was your host machine. IIUC this change will only improve >> performance if the host tcg backend is able to implement TCG vector >> ops in terms of vector ops on the host. > > Tried it on i5 650 which has: sse sse2 ssse3 sse4_1 sse4_2. I assume x86_64 > should be supported but not sure what are the CPU requirements. Not quite. I only support avx1 and later. I thought about supporting sse4 and later (that's the minimum with all of the instructions that do what we need), but there is only one cpu generation with sse4 and without avx1, and avx1 is already 8 years old. r~
Re: [Qemu-devel] [RFC PATCH 1/7] virtio: Add shared memory capability
On 12/10/18 11:31 AM, Dr. David Alan Gilbert (git) wrote:
From: "Dr. David Alan Gilbert"
Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG'
and the data structure 'virtio_pci_shm_cap' to go with it.
They allow defining shared memory regions with sizes and offsets
of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.
Signed-off-by: Dr. David Alan Gilbert
---
hw/virtio/virtio-pci.c | 20
include/standard-headers/linux/virtio_pci.h | 9 +
2 files changed, 29 insertions(+)
+++ b/include/standard-headers/linux/virtio_pci.h
@@ -113,6 +113,8 @@
#define VIRTIO_PCI_CAP_DEVICE_CFG 4
/* PCI configuration access */
#define VIRTIO_PCI_CAP_PCI_CFG5
+/* Additional shared memory capability */
+#define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
/* This is the PCI capability header: */
struct virtio_pci_cap {
@@ -163,6 +165,13 @@ struct virtio_pci_cfg_cap {
uint8_t pci_cfg_data[4]; /* Data for BAR access. */
};
+struct virtio_pci_shm_cap {
+ struct virtio_pci_cap cap;
+ uint32_t offset_hi; /* Most sig 32 bits of offset */
+ uint32_t length_hi; /* Most sig 32 bits of length */
+uint8_t id;/* To distinguish shm chunks */
TAB damage.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH] cpus.c: Fix race condition in cpu_stop_current()
Hi again,
On 12/7/18 4:59 PM, Peter Maydell wrote:
> We use cpu_stop_current() to ensure the current CPU has stopped
> from places like qemu_system_reset_request(). Unfortunately its
> current implementation has a race. It calls qemu_cpu_stop(),
> which sets cpu->stopped to true even though the CPU hasn't
> actually stopped yet. The main thread will look at the flags
> set by qemu_system_reset_request() and call pause_all_vcpus().
> pause_all_vcpus() waits for every cpu to have cpu->stopped true,
> so it can continue (and we will start the system reset operation)
> before the vcpu thread has got back to its top level loop.
>
> Instead, just set cpu->stop and call cpu_exit(). This will
> cause the vcpu to exit back to the top level loop, and there
> (as part of the wait_io_event code) it will call qemu_cpu_stop().
>
> This fixes bugs where the reset request appeared to be ignored
> or the CPU misbehaved because the reset operation started
> to change vcpu state while the vcpu thread was still using it.
>
> Signed-off-by: Peter Maydell
> ---
> We discussed this a little while back:
> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00154.html
> and Jaap reported a bug which I suspect of being the same thing:
> https://lists.gnu.org/archive/html/qemu-discuss/2018-10/msg00014.html
>
> Annoyingly I have lost the test case that demonstrated this
> race, but I analysed it at the time and this should definitely
> fix it. I have opted not to try to address any of the other
> possible cleanup here (eg vm_stop() has a potential similar
> race if called from a vcpu thread I suspect), since it gets
> pretty tangled.
>
> Jaap: could you test whether this patch fixes the issue you
> were seeing, please?
> ---
> cpus.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index 0ddeeefc14f..b09b7027126 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -2100,7 +2100,8 @@ void qemu_init_vcpu(CPUState *cpu)
> void cpu_stop_current(void)
> {
> if (current_cpu) {
> -qemu_cpu_stop(current_cpu, true);
> +current_cpu->stop = true;
> +cpu_exit(current_cpu);
> }
> }
>
>
The patch also fixed the issue on my production system. Thanks!
regards,
Jaap
Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 0/6] target/ppc: convert VMX instructions to use TCG vector operations
On Mon, 10 Dec 2018, David Gibson wrote: On Mon, Dec 10, 2018 at 01:33:53AM +0100, BALATON Zoltan wrote: On Fri, 7 Dec 2018, Mark Cave-Ayland wrote: This patchset is an attempt at trying to improve the VMX (Altivec) instruction performance by making use of the new TCG vector operations where possible. This is very welcome, thanks for doing this. In order to use TCG vector operations, the registers must be accessible from cpu_env whilst currently they are accessed via arrays of static TCG globals. Patches 1-3 are therefore mechanical patches which introduce access helpers for FPR, AVR and VSR registers using the supplied TCGv_i64 parameter. Have you tried some benchmarks or tests to measure the impact of these changes? I've tried the (very unscientific) benchmarks I've written about before here: http://lists.nongnu.org/archive/html/qemu-ppc/2018-07/msg00261.html (which seem to use AltiVec/VMX instructions but not sure which) on mac99 with MorphOS and I could not see any performance increase. I haven't run enough tests but results with or without this series on master were mostly the same within a few percents, and sometimes even seen lower performance with these patches than without. I haven't tried to find out why (no time for that now) so can't really draw any conclusions from this. I'm also not sure if I've actually tested what you've changed or these use instructions that your patches don't optimise yet, or the changes I've seen were just normal changes between runs; but I wonder if the increased number of temporaries could result in lower performance in some cases? What was your host machine. IIUC this change will only improve performance if the host tcg backend is able to implement TCG vector ops in terms of vector ops on the host. Tried it on i5 650 which has: sse sse2 ssse3 sse4_1 sse4_2. I assume x86_64 should be supported but not sure what are the CPU requirements. In addition, this series only converts a subset of the integer and logical vector instructions. If your testcase is mostly floating point (vectored or otherwise), it will still be softfloat and so not see any speedup. Yes, I don't really know what these tests use but I think "lame" test is mostly floating point but tried with "lame_vmx" which should at least use some vector ops and "mplayer -benchmark" test is more vmx dependent based on my previous profiling and testing with hardfloat but I'm not sure. (When testing these with hardfloat I've found that lame was benefiting from hardfloat but mplayer wasn't and more VMX related functions showed up with mplayer so I assumed it's more VMX bound.) I've tried to do some profiling again to find out what's used but I can't get good results with the tools I have (oprofile stopped working since I've updated my machine and Linux perf provides results that are hard to interpret for me, haven't tried if gprof would work now it didn't before) but I've seen some vector related helpers in the profile so at least some vector ops are used. The "helper_vperm" came up top at about 11th (not sure where is it called from), other vector helpers were lower. I don't remember details now but previously when testing hardfloat I've written this: "I've looked at vperm which came out top in one of the profiles I've taken and on little endian hosts it has the loop backwards and also accesses vector elements from end to front which I wonder may be enough for the compiler to not be able to optimise it? But I haven't checked assembly. The altivec dependent mplayer video decoding test did not change much with hardfloat, it took 98% compared to master so likely altivec is dominating here." (Although this was with the PPC specific vector helpers before VMX patch so not sure if this is still relevant.) The top 10 in profile were still related to low level memory access and MMU management stuff as I've found before: http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg03609.html http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg03704.html I think implementing i2c for mac99 may help this and some other optimisations may also be possible but I don't know enough about these to try that. It also looks like with --enable-debug something is always flusing tlb and blowing away tb caches so these will be top in profile and likely dominate runtime so can't really use profile to measure impact of VMX patch. Without --enable-debug I can't get call graphs so can't get useful profile. I think I've looked at this before as well but can't remember now which check enabled by --enable-debug is responsible for constant tb cache flush and if that could be avoided. I just don't use --enable-debug since unless need to debug somthing. Maybe the PPC softmmu should be reviewed and optimised by someone who knows it... Regards, BALATON Zoltan
Re: [Qemu-devel] Help needed: test-qht-par hangs on Travis
On Mon, Dec 10, 2018 at 15:47:15 -0200, Eduardo Habkost wrote: > On Sun, Dec 09, 2018 at 05:27:38PM -0500, Emilio G. Cota wrote: > > On Fri, Dec 07, 2018 at 18:41:07 -0200, Eduardo Habkost wrote: > > > I've noticed QEMU Travis builds are failing recently, and they > > > seem to happen only on the --enable-gprof jobs. I have enabled > > > V=1 and noticed that the jobs are hanging inside test-qht-par. > > > > > > Example here (look for "/qht/parallel/2threads-0%updates-1s"): > > > > > > https://travis-ci.org/ehabkost/qemu-hacks/jobs/465081311 > > > > > > Does anybody have any idea why? > > > > So if I read that output correctly, it seems that the second > > test in qht-par never completes. > > > > Enabling gprof and gcov (as in that build) should just lower > > the throughput of the benchmark (test-qht-par invokes qht-bench), > [...] > > Unrelated question: is there a specific reason why test-qht-par > is written in C using gtest, instead of being just a shell script > that runs qht-bench? I didn't know how to integrate a shell script with gtester, so I went with a C program. There's a possibility that the use of system(3) here is what's causing the problem. Can you try running the following branch on travis? https://github.com/cota/qemu/tree/test-qht-par I moved most of qht-bench into qht-bench.inc.c, so that both qht-bench.c and test-qht-par.c can use it. This gets rid of the use of system(3) in test-qht-par.c. Thanks, Emilio
Re: [Qemu-devel] [PATCH] target/arm: Implement VI/VF bits
On Mon, 10 Dec 2018 at 18:38, Andrew Walbran wrote: > > From: Wedson Almeida Filho > > This lets hypervisors running at EL2 inject virtual interrupts into guest VMs. > > Signed-off-by: Wedson Almeida Filho > Signed-off-by: Andrew Walbran Hi; thanks for this patch. I'm a bit confused -- can you explain what the patch provides that isn't already implemented by commit 89430fc6f80a5aef1d4cb ? thanks -- PMM
Re: [Qemu-devel] [RFC PATCH 0/7] virtio-fs: shared file system for virtual machines3
Patchew URL: https://patchew.org/QEMU/20181210173151.16629-1-dgilb...@redhat.com/ 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 time make docker-test-mingw@fedora SHOW_ENV=1 J=8 === TEST SCRIPT END === CC ui/gtk.o CC chardev/char.o CC chardev/char-console.o /tmp/qemu-test/src/hw/virtio/virtio-pci.c:1167:12: error: 'virtio_pci_add_shm_cap' defined but not used [-Werror=unused-function] static int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, ^~ cc1: all warnings being treated as errors The full log is available at http://patchew.org/logs/20181210173151.16629-1-dgilb...@redhat.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 3/3] vfio-pci: Add FAILOVER_PRIMARY_CHANGED event to shorten downtime during failover
On 12/10/2018 9:31 AM, Michael S. Tsirkin wrote:
On Mon, Dec 10, 2018 at 11:15:48AM -0500, Venu Busireddy wrote:
From: Si-Wei Liu
When a VF is hotplugged into the guest, datapath switching will be
performed immediately, which is sub-optimal in terms of timing, and
could end up with substantial network downtime. One of ways to shorten
this downtime is to switch the datapath only after the VF is seen to get
enabled by guest, indicated by the bus master bit in VF's PCI config
space getting enabled. The FAILOVER_PRIMARY_CHANGED event is emitted
at that time to indicate this condition. Then management stack can kick
off datapath switching upon receiving the event.
Signed-off-by: Si-Wei Liu
Signed-off-by: Venu Busireddy
As management stacks can lose events, it's necessary
to also have a query command to check device status.
Hmm, makes sense. Will do.
-Siwei
---
hw/vfio/pci.c | 57 +
qapi/net.json | 26 ++
2 files changed, 83 insertions(+)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index ce1f33c..ea24ca2 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -34,6 +34,7 @@
#include "pci.h"
#include "trace.h"
#include "qapi/error.h"
+#include "qapi/qapi-events-net.h"
#define MSIX_CAP_LENGTH 12
@@ -42,6 +43,7 @@
static void vfio_disable_interrupts(VFIOPCIDevice *vdev);
static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
+static void vfio_failover_notify(VFIOPCIDevice *vdev, bool status);
/*
* Disabling BAR mmaping can be slow, but toggling it around INTx can
@@ -1170,6 +1172,8 @@ void vfio_pci_write_config(PCIDevice *pdev,
{
VFIOPCIDevice *vdev = PCI_VFIO(pdev);
uint32_t val_le = cpu_to_le32(val);
+bool may_notify = false;
+bool master_was = false;
trace_vfio_pci_write_config(vdev->vbasedev.name, addr, val, len);
@@ -1180,6 +1184,14 @@ void vfio_pci_write_config(PCIDevice *pdev,
__func__, vdev->vbasedev.name, addr, val, len);
}
+/* Bus Master Enabling/Disabling */
+if (pdev->failover_primary && current_cpu &&
+range_covers_byte(addr, len, PCI_COMMAND)) {
+master_was = !!(pci_get_word(pdev->config + PCI_COMMAND) &
+PCI_COMMAND_MASTER);
+may_notify = true;
+}
+
/* MSI/MSI-X Enabling/Disabling */
if (pdev->cap_present & QEMU_PCI_CAP_MSI &&
ranges_overlap(addr, len, pdev->msi_cap, vdev->msi_cap_size)) {
@@ -1235,6 +1247,14 @@ void vfio_pci_write_config(PCIDevice *pdev,
/* Write everything to QEMU to keep emulated bits correct */
pci_default_write_config(pdev, addr, val, len);
}
+
+if (may_notify) {
+bool master_now = !!(pci_get_word(pdev->config + PCI_COMMAND) &
+ PCI_COMMAND_MASTER);
+if (master_was != master_now) {
+vfio_failover_notify(vdev, master_now);
+}
+}
}
/*
It's very easy to have guest trigger a high load of events by playing
with the bus master enable bits. How about instead sending an event
that just says "something changed" without the current status and have
management issue a query command to check the status. QEMU then does not
need to re-send an event until management issues a query command.
@@ -2801,6 +2821,17 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice
*vdev)
vdev->req_enabled = false;
}
+static void vfio_failover_notify(VFIOPCIDevice *vdev, bool status)
+{
+PCIDevice *pdev = >pdev;
+const char *n;
+gchar *path;
+
+n = pdev->qdev.id ? pdev->qdev.id : vdev->vbasedev.name;
+path = object_get_canonical_path(OBJECT(vdev));
+qapi_event_send_failover_primary_changed(!!n, n, path, status);
+}
+
static void vfio_realize(PCIDevice *pdev, Error **errp)
{
VFIOPCIDevice *vdev = PCI_VFIO(pdev);
@@ -3109,10 +3140,36 @@ static void vfio_instance_finalize(Object *obj)
vfio_put_group(group);
}
+static void vfio_exit_failover_notify(VFIOPCIDevice *vdev)
+{
+PCIDevice *pdev = >pdev;
+
+/*
+ * Guest driver may not get the chance to disable bus mastering
+ * before the device object gets to be unrealized. In that event,
+ * send out a "disabled" notification on behalf of guest driver.
+ */
+if (pdev->failover_primary &&
+pdev->bus_master_enable_region.enabled) {
+vfio_failover_notify(vdev, false);
+}
+}
+
With the idea above this won't be necessary anymore.
static void vfio_exitfn(PCIDevice *pdev)
{
VFIOPCIDevice *vdev = PCI_VFIO(pdev);
+/*
+ * During the guest reboot sequence, it is sometimes possible that
+ * the guest may not get sufficient time to complete the entire driver
+ * removal sequence, near the end of which a PCI config space write to
+ * disable bus mastering can be intercepted by device. In such cases,
+ * the FAILOVER_PRIMARY_CHANGED "disable"
Re: [Qemu-devel] [PATCH 2/3] virtio_net: Add support for "Data Path Switching" during Live Migration.
On 12/10/2018 9:41 AM, Michael S. Tsirkin wrote:
On Mon, Dec 10, 2018 at 11:15:47AM -0500, Venu Busireddy wrote:
Added two new events, FAILOVER_PLUG_PRIMARY and FAILOVER_UNPLUG_PRIMARY.
The first is emitted when the guest negotiates the F_STANDBY feature
bit. The second is emitted when the virtio_net driver is removed, either
manually or as a result of guest reboot.
So the names mean "should plug" and "should unplug" right?
Sounds about right, but management stack may ignore the message if not
going to plug in the primary device.
It seems inelegant to tell upper layers what they should do.
After all they are upper layers because presumably
there is a policy question as opposed to a mechanism.
Can we expose information without telling managmenent what to do?
Is it just the name itself that is offensive? The information exposed to
upper layer is just that time is ready to plug/unplug the paired device.
Would the names FAILOVER_STANDY_SET and FAILOVER_STANDY_CLEAR fit your
expectation?
Alternatively, is there a real need to unplug the device completely?
Would it work to just hide the device from guest instead? QEMU can do
it itself and this is the direction that Sameeh has been taking.
See below in the cover letter:
3. While the hidden device model (viz. coupled device model) is still
being explored for automatic hot plugging (QEMU) and automatic datapath
switching (host-kernel), this series provides a supplemental set
of interfaces if management software wants to drive the SR-IOV live
migration on its own. It should not conflict with the hidden device
model but just offers simplicity of implementation.
As said this is a supplemental implementation that involves and
leverages management software before we can converge to the ideal
hidden/coupled device model.
As I understand it, we're still exploring the best way to handle the
datapath (MAC filter) switching problem for the hidden (coupled) device
model, right. Even if we can hide the device there's still need to
inform upper layer for datapath switching that is currently being
handled in userspace management stack. I think the best fit for the
hidden (coupled) model is that all datapath switching can be handled
automatically and transparently in the kernel without needing to notify
userspace and depend on userspace reaction.
Management stack can use these
events to determine when to plug/unplug the VF device to/from the guest.
Also, the Virtual Functions will be automatically removed from the guest
if the guest is rebooted. To properly identify the VFIO devices that
must be removed, a new property named "x-failover-primary" is added to
the vfio-pci devices. Only the vfio-pci devices that have this property
enabled are removed from the guest upon reboot.
If this property needs to be set by management then
it must not start with "x-" - that prefix means
"unstable do not use from external tools".
Sure, will remove "x-" (sorry, missed to correct it before sending out
publicly).
Thanks,
-Siwei
Signed-off-by: Venu Busireddy
---
hw/acpi/pcihp.c| 27 ++
hw/net/virtio-net.c| 23 ++
hw/vfio/pci.c | 3 +++
hw/vfio/pci.h | 1 +
include/hw/pci/pci.h | 1 +
include/hw/virtio/virtio-net.h | 4
qapi/net.json | 44 ++
7 files changed, 103 insertions(+)
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 80d42e1..2a3ffd3 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -176,6 +176,25 @@ static void acpi_pcihp_eject_slot(AcpiPciHpState *s,
unsigned bsel, unsigned slo
}
}
+static void acpi_pcihp_cleanup_failover_primary(AcpiPciHpState *s, int bsel)
+{
+BusChild *kid, *next;
+PCIBus *bus = acpi_pcihp_find_hotplug_bus(s, bsel);
+
+if (!bus) {
+return;
+}
+QTAILQ_FOREACH_SAFE(kid, >qbus.children, sibling, next) {
+DeviceState *qdev = kid->child;
+PCIDevice *pdev = PCI_DEVICE(qdev);
+int slot = PCI_SLOT(pdev->devfn);
+
+if (pdev->failover_primary) {
+s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
+}
+}
+}
+
static void acpi_pcihp_update_hotplug_bus(AcpiPciHpState *s, int bsel)
{
BusChild *kid, *next;
@@ -207,6 +226,14 @@ static void acpi_pcihp_update(AcpiPciHpState *s)
int i;
for (i = 0; i < ACPI_PCIHP_MAX_HOTPLUG_BUS; ++i) {
+/*
+ * Set the acpi_pcihp_pci_status[].down bits of all the
+ * failover_primary devices so that the devices are ejected
+ * from the guest. We can't use the qdev_unplug() as well as the
+ * hotplug_handler to unplug the devices, because the guest may
+ * not be in a state to cooperate.
+ */
+acpi_pcihp_cleanup_failover_primary(s, i);
acpi_pcihp_update_hotplug_bus(s, i);
}
}
diff --git
Re: [Qemu-devel] [PATCH v5] qemu-img info lists bitmap directory entries
On 12/10/18 12:50 PM, Vladimir Sementsov-Ogievskiy wrote:
10.12.2018 21:09, Andrey Shinkevich wrote:
In the 'Format specific information' section of the 'qemu-img info'
command output, the supplemental information about existing QCOW2
bitmaps will be shown, such as a bitmap name, flags and granularity:
[...]
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4270,6 +4270,19 @@ static ImageInfoSpecific
*qcow2_get_specific_info(BlockDriverState *bs)
.refcount_bits = s->refcount_bits,
};
} else if (s->qcow_version == 3) {
+bool has_bitmaps;
+Qcow2BitmapInfoList *bitmaps;
+Error *local_err = NULL;
+
+bitmaps = qcow2_get_bitmap_info_list(bs, _err);
+if (local_err) {
+/* TODO: Report the Error up to the caller when implemented */
+error_free(local_err);
+/* The 'bitmaps' empty list designates a failure to get info */
+has_bitmaps = true;
I think you got it backwards. I would prefer has_bitmaps = false on
error,...
+} else {
+has_bitmaps = !!bitmaps;
...and an empty list when you successfully determined that there are no
bitmaps.
+++ b/qapi/block-core.json
@@ -69,6 +69,11 @@
# @encrypt: details about encryption parameters; only set if image
# is encrypted (since 2.10)
#
+# @bitmaps: list of qcow2 bitmaps details; the empty list designates
+# "fail to load bitmaps" if it is passed to the caller or
+# "no bitmaps" otherwise;
+# unsupported for the format QCOW2 v2 (since 4.0)
For me, it looks simpler to declare alternative approach, assuming that absence
of the field means error, like this:
@bitmaps: optional field with uncommon semantics:
Absence of the field means that bitmaps info query failed (which
doesn't
imply the whole query failure).
If the field exists in query results, there were no errors, and it
represents
list of qcow2 bitmaps details. So, successful result will always
use empty
list to show that there are no bitmaps.
Note: bitmaps are not supported before QCOW2 v3, so for elder
versions
@bitmaps will always be an empty list.
I'd prefer:
@bitmaps: A list of qcow2 bitmap details (possibly empty, such as for v2
images which do not support bitmaps). Absent if bitmap
information could not be obtained. (since 4.0)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fixes i386 xchgq test
On 12/10/18 1:26 PM, Philippe Mathieu-Daudé wrote: > Hi Fabrice, > > On Fri, Dec 7, 2018 at 5:06 PM wrote: >> >> As "xchg" reads and writes both operands, the "+m" is required to avoid >> undefined behavior on -O2 compilation. >> >> Signed-off-by: Fabrice Desclaux Reviewed-by: Richard Henderson >> --- >> tests/tcg/i386/test-i386.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/tcg/i386/test-i386.c b/tests/tcg/i386/test-i386.c >> index a29b41e764..18d5609665 100644 >> --- a/tests/tcg/i386/test-i386.c >> +++ b/tests/tcg/i386/test-i386.c >> @@ -1137,7 +1137,7 @@ void test_xchg(void) >> TEST_XCHG(xchgb, "b", "+q"); >> >> #if defined(__x86_64__) >> -TEST_XCHG(xchgq, "", "=m"); >> +TEST_XCHG(xchgq, "", "+m"); >> #endif >> TEST_XCHG(xchgl, "k", "+m"); >> TEST_XCHG(xchgw, "w", "+m"); >> -- >> 2.19.2 > > All QEMU patches has to go via the qemu-devel@nongnu.org list (you > only sent it to the trivial list). > I'm also Cc'ing the maintainers: Thanks, Phil. r~
Re: [Qemu-devel] [PATCH 2/3] virtio_net: Add support for "Data Path Switching" during Live Migration.
On 12/10/18 10:15 AM, Venu Busireddy wrote:
Added two new events, FAILOVER_PLUG_PRIMARY and FAILOVER_UNPLUG_PRIMARY.
The first is emitted when the guest negotiates the F_STANDBY feature
bit. The second is emitted when the virtio_net driver is removed, either
manually or as a result of guest reboot. Management stack can use these
events to determine when to plug/unplug the VF device to/from the guest.
Also, the Virtual Functions will be automatically removed from the guest
if the guest is rebooted. To properly identify the VFIO devices that
must be removed, a new property named "x-failover-primary" is added to
the vfio-pci devices. Only the vfio-pci devices that have this property
enabled are removed from the guest upon reboot.
Signed-off-by: Venu Busireddy
---
+++ b/qapi/net.json
@@ -683,3 +683,47 @@
##
{ 'event': 'NIC_RX_FILTER_CHANGED',
'data': { '*name': 'str', 'path': 'str' } }
+
+##
+# @FAILOVER_PLUG_PRIMARY:
+#
+# Emitted when the guest successfully loads the driver after the STANDBY
+# feature bit is negotiated.
+#
+# @device: Indicates the virtio_net device.
+#
+# @path: Indicates the device path.
+#
+# Since: 3.0
You've missed 3.0, and even 3.1. This should be 4.0.
+#
+# Example:
+#
+# <- {"timestamp": {"seconds": 1432121972, "microseconds": 744001},
+# "event": "FAILOVER_PLUG_PRIMARY",
+# "data": {"device": "net0", "path":
"/machine/peripheral/net0/virtio-backend"} }
+#
+##
+{ 'event': 'FAILOVER_PLUG_PRIMARY',
+ 'data': {'*device': 'str', 'path': 'str'} }
+
+##
+# @FAILOVER_UNPLUG_PRIMARY:
+#
+# Emitted when the guest resets the virtio_net driver.
+# The reset can be the result of either unloading the driver or a reboot.
+#
+# @device: Indicates the virtio_net device.
+#
+# @path: Indicates the device path.
+#
+# Since: 3.0
and again
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fixes i386 xchgq test
Hi Fabrice, On Fri, Dec 7, 2018 at 5:06 PM wrote: > > As "xchg" reads and writes both operands, the "+m" is required to avoid > undefined behavior on -O2 compilation. > > Signed-off-by: Fabrice Desclaux > --- > tests/tcg/i386/test-i386.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/tcg/i386/test-i386.c b/tests/tcg/i386/test-i386.c > index a29b41e764..18d5609665 100644 > --- a/tests/tcg/i386/test-i386.c > +++ b/tests/tcg/i386/test-i386.c > @@ -1137,7 +1137,7 @@ void test_xchg(void) > TEST_XCHG(xchgb, "b", "+q"); > > #if defined(__x86_64__) > -TEST_XCHG(xchgq, "", "=m"); > +TEST_XCHG(xchgq, "", "+m"); > #endif > TEST_XCHG(xchgl, "k", "+m"); > TEST_XCHG(xchgw, "w", "+m"); > -- > 2.19.2 All QEMU patches has to go via the qemu-devel@nongnu.org list (you only sent it to the trivial list). I'm also Cc'ing the maintainers: $ ./scripts/get_maintainer.pl -f tests/tcg/i386/test-i386.c Paolo Bonzini (maintainer:X86) Richard Henderson (maintainer:X86) Eduardo Habkost (maintainer:X86) qemu-devel@nongnu.org (open list:All patches CC here) Regards, Phil.
Re: [Qemu-devel] [RFC PATCH 3/6] target/ppc: introduce get_cpu_vsr{l, h}() and set_cpu_vsr{l, h}() helpers for VSR register access
On 12/7/18 2:56 AM, Mark Cave-Ayland wrote:
> +static inline void get_vsr(TCGv_i64 dst, int n)
> +{
> +tcg_gen_ld_i64(dst, cpu_env, offsetof(CPUPPCState, vsr[n]));
> +}
> +
> +static inline void set_vsr(int n, TCGv_i64 src)
> +{
> +tcg_gen_st_i64(src, cpu_env, offsetof(CPUPPCState, vsr[n]));
> +}
Why isn't this helper still using cpu_vsr[n]?
r~
[Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device
If the maximum number of devices allowed on a bus is 1 and a device which is plugged into the bus is subsequently unplugged, attempting to replug the device fails with error "Bus 'xxx' does not support hotplugging". The "error" is detected in the qbus_is_full(BusState *bus) function (qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The root of the problem is that the bus->max_index is not decremented when a device is unplugged from the bus. This patch fixes that problem. Signed-off-by: Tony Krowiak --- hw/core/qdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 6b3cc55b27c2..b35b0bf27925 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState *child) snprintf(name, sizeof(name), "child[%d]", kid->index); QTAILQ_REMOVE(>children, kid, sibling); +bus->max_index--; + /* This gives back ownership of kid->child back to us. */ object_property_del(OBJECT(bus), name, NULL); object_unref(OBJECT(kid->child)); -- 2.7.4
[Qemu-devel] [RFC] block: Is name of BlockBackend deprecated with -blockdev parameter?
Hello, I'm trying to switch from -drive parameter to -blockdev + -device and having problems. Looks like with this option I have no way to set the name of created BlockBackend, but some QMP and HMP commands are trying to find blk with blk_by_name() and fail to locate my device (e.g. hmp_commit, qmp_x_bloc_latency_histogram_set ...). Was it intentional and BB names are going to be deprecated? This also seems to be a the case for all block devices hotplugged with QMP as they use the same code path. As far as I understand all named backends are stored in monitor_block_backends list, but I can't get what is the point of having this list, and why parse_drive() function doesn't call monitor_add_blk() like blockdev_init() does with -drive option. Can someone give me a hint on this? I also noticed that some commands fallback to search by qdev_id or BDS->node_name, but at the creation time (both in bdrv_assing_node_name and monitor_add_blk) it is already checked that names are unique across these namespaces so may be it would be useful to introduce generic search function? Thanks, Anton
Re: [Qemu-devel] [RFC PATCH 6/6] target/ppc: convert vaddu[b, h, w, d] and vsubu[b, h, w, d] over to use vector operations
On 12/7/18 2:56 AM, Mark Cave-Ayland wrote: > Signed-off-by: Mark Cave-Ayland > --- > target/ppc/helper.h | 8 > target/ppc/int_helper.c | 7 --- > target/ppc/translate/vmx-impl.inc.c | 16 > 3 files changed, 8 insertions(+), 23 deletions(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] Help needed: test-qht-par hangs on Travis
On Mon, Dec 10, 2018 at 03:34:27PM -0200, Eduardo Habkost wrote:
> On Mon, Dec 10, 2018 at 12:07:20PM -0500, Emilio G. Cota wrote:
> > On Mon, Dec 10, 2018 at 14:36:01 -0200, Eduardo Habkost wrote:
> > > On Sun, Dec 09, 2018 at 05:27:38PM -0500, Emilio G. Cota wrote:
> > > > Can you try re-running the test, after applying the appended patch?
> > > > (It disables the "resize" thread.)
> > >
> > > It is running right now, here:
> > > https://travis-ci.org/ehabkost/qemu-hacks/jobs/466074591
> > >
> > > >
> > > > Also, does it reliably hang on Travis, or are these hangs
> > > > intermittent?
> > >
> > > It can be reproduced reliably. qemu.git builds are failing since
> > > Thursday:
> > > https://travis-ci.org/qemu/qemu/builds
> >
> > I see the build you launched timed out. Can you try the following
> > patch (after discarding the previous one)? Let's see if just by
> > disabling the second test we can get the build to move ahead.
>
> I will try it. I'm not sure yet if it's the first or the second
> test case timing out. Maybe the "OK\n" we see in the log file is
> from another process running in parallel.
Yeah, I think the first test case is the one hanging:
https://travis-ci.org/ehabkost/qemu-hacks/jobs/466074591#L7741
>
>
> >
> > Thanks,
> >
> > E.
> > ---
> > diff --git a/tests/test-qht-par.c b/tests/test-qht-par.c
> > index d8a83caf5c..a916c91ccb 100644
> > --- a/tests/test-qht-par.c
> > +++ b/tests/test-qht-par.c
> > @@ -46,7 +46,6 @@ int main(int argc, char *argv[])
> >
> > if (g_test_quick()) {
> > g_test_add_func("/qht/parallel/2threads-0%updates-1s",
> > test_2th0u1s);
> > -g_test_add_func("/qht/parallel/2threads-20%updates-1s",
> > test_2th20u1s);
> > } else {
> > g_test_add_func("/qht/parallel/2threads-0%updates-5s",
> > test_2th0u5s);
> > g_test_add_func("/qht/parallel/2threads-20%updates-5s",
> > test_2th20u5s);
>
> --
> Eduardo
--
Eduardo
Re: [Qemu-devel] [RFC PATCH 5/6] target/ppc: convert VMX logical instructions to use vector operations
On 12/7/18 2:56 AM, Mark Cave-Ayland wrote: > Signed-off-by: Mark Cave-Ayland > --- > target/ppc/translate.c | 1 + > target/ppc/translate/vmx-impl.inc.c | 64 > ++--- > 2 files changed, 40 insertions(+), 25 deletions(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [QEMU-devel][PATCH v2] aio-posix: Fix concurrent aio_poll/set_fd_handler.
On Thu, Dec 06, 2018 at 11:14:23AM +0100, remy.n...@blade-group.com wrote:
> +if (is_new) {
> +new_node->pfd.fd = fd;
> +} else {
> +deleted = aio_remove_fd_handler(ctx, node);
> +new_node->pfd = node->pfd;
Does this drop revents? Imagine revents has bits set, then
aio_remove_fd_handler() will clear them and new_node->pfd = node->pfd is
too late to preserve them.
signature.asc
Description: PGP signature
Re: [Qemu-devel] [RFC PATCH 4/6] target/ppc: switch FPR, VMX and VSX helpers to access data directly from cpu_env
On 12/7/18 2:56 AM, Mark Cave-Ayland wrote: > Instead of accessing the FPR, VMX and VSX registers through static arrays of > TCGv_i64 globals, remove them and change the helpers to load/store data > directly > within cpu_env. > > Signed-off-by: Mark Cave-Ayland > --- > target/ppc/translate.c | 59 > ++ > 1 file changed, 16 insertions(+), 43 deletions(-) Reviewed-by: Richard Henderson Note however, that there are other steps that you must add here before using vector operations in the next patch: (1a) The fpr and vsr arrays must be merged, since fpr[n] == vsrh[n]. If this isn't done, then you simply cannot apply one operation to two disjoint memory blocks. (1b) The vsr and avr arrays should be merged, since vsr[32+n] == avr[n]. This is simply tidiness, matching the layout to the architecture. These steps will modify gdbstub.c, machine.c, and linux-user/. (2) The vsr array needs to be QEMU_ALIGN(16). See target/arm/cpu.h. We assert that the host addresses are 16 byte aligned, so that we can eventually use Altivec/VSX in tcg/ppc/. r~
Re: [Qemu-devel] [PATCH v5] qemu-img info lists bitmap directory entries
10.12.2018 21:09, Andrey Shinkevich wrote:
> In the 'Format specific information' section of the 'qemu-img info'
> command output, the supplemental information about existing QCOW2
> bitmaps will be shown, such as a bitmap name, flags and granularity:
>
[...]
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -4270,6 +4270,19 @@ static ImageInfoSpecific
> *qcow2_get_specific_info(BlockDriverState *bs)
> .refcount_bits = s->refcount_bits,
> };
> } else if (s->qcow_version == 3) {
> +bool has_bitmaps;
> +Qcow2BitmapInfoList *bitmaps;
> +Error *local_err = NULL;
> +
> +bitmaps = qcow2_get_bitmap_info_list(bs, _err);
> +if (local_err) {
> +/* TODO: Report the Error up to the caller when implemented */
> +error_free(local_err);
> +/* The 'bitmaps' empty list designates a failure to get info */
> +has_bitmaps = true;
> +} else {
> +has_bitmaps = !!bitmaps;
> +}
> *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
> .compat = g_strdup("1.1"),
> .lazy_refcounts = s->compatible_features &
> @@ -4279,6 +4292,8 @@ static ImageInfoSpecific
> *qcow2_get_specific_info(BlockDriverState *bs)
> QCOW2_INCOMPAT_CORRUPT,
> .has_corrupt= true,
> .refcount_bits = s->refcount_bits,
> +.has_bitmaps= has_bitmaps,
> +.bitmaps= bitmaps,
> };
> } else {
> /* if this assertion fails, this probably means a new version was
[..]
> index d4fe710..53820a5 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -69,6 +69,11 @@
> # @encrypt: details about encryption parameters; only set if image
> # is encrypted (since 2.10)
> #
> +# @bitmaps: list of qcow2 bitmaps details; the empty list designates
> +# "fail to load bitmaps" if it is passed to the caller or
> +# "no bitmaps" otherwise;
> +# unsupported for the format QCOW2 v2 (since 4.0)
For me, it looks simpler to declare alternative approach, assuming that absence
of the field means error, like this:
@bitmaps: optional field with uncommon semantics:
Absence of the field means that bitmaps info query failed (which
doesn't
imply the whole query failure).
If the field exists in query results, there were no errors, and it
represents
list of qcow2 bitmaps details. So, successful result will always use
empty
list to show that there are no bitmaps.
Note: bitmaps are not supported before QCOW2 v3, so for elder
versions
@bitmaps will always be an empty list.
The main question here: is it a first time, we are doing something like this?
If not,
we must go the existing way.
I've found the only one similar thing:
in qapi/misc.json:
# If @unavailable-features is an empty list, the CPU model is
# runnable using the current host and machine-type.
# If @unavailable-features is not present, runnability
# information for the CPU is not available.
it's not about error, however..
Interesting what is the common (most common) behavior around
empty-list/absent-parameter?
Aha, one point to my semantics:
we can define required field, without '*', and it implies that there should not
be any errors,
and we don't have extra options (only empty list is possible to show absence of
elements).
and than, '*' shows possibility of errors (if described in spec).
and with your semantics, if we want to say in general, that empty-list = error,
we'll need to
use '*' for all fields, even for thous without possible errors.
(of course, we actually can not say something in general, because, I'm afraid,
that we currently
have mixed semantics around empty lists)
> +#
> # Since: 1.7
> ##
> { 'struct': 'ImageInfoSpecificQCow2',
> @@ -77,7 +82,8 @@
> '*lazy-refcounts': 'bool',
> '*corrupt': 'bool',
> 'refcount-bits': 'int',
> - '*encrypt': 'ImageInfoSpecificQCow2Encryption'
> + '*encrypt': 'ImageInfoSpecificQCow2Encryption',
> + '*bitmaps': ['Qcow2BitmapInfo']
> } }
>
> ##
> @@ -454,6 +460,41 @@
> 'status': 'DirtyBitmapStatus'} }
>
--
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH v6 18/27] qapi: add an error in case a discriminator is conditionnal
Hi
On Thu, Dec 6, 2018 at 8:25 PM Markus Armbruster wrote:
>
> Marc-André Lureau writes:
>
> > Making a discriminator conditonal doesn't make much sense.
>
> Good point (so easy to overlook!), but why first add the 'if' feature
> broken that way in PATCH 16, then fix it up in PATCH 18?
Not sure, I guess I found out after. Feel free to squash
>
> >Instead,
> > the union could be made conditional.
>
> What do you mean by that?
That the alternative is probably to make the whole union conditional
>
> > Signed-off-by: Marc-André Lureau
> > ---
> > scripts/qapi/common.py | 11 +--
> > tests/Makefile.include | 1 +
> > .../flat-union-invalid-if-discriminator.err | 1 +
> > .../flat-union-invalid-if-discriminator.exit| 1 +
> > .../flat-union-invalid-if-discriminator.json| 17 +
> > .../flat-union-invalid-if-discriminator.out | 0
> > 6 files changed, 29 insertions(+), 2 deletions(-)
> > create mode 100644
> > tests/qapi-schema/flat-union-invalid-if-discriminator.err
> > create mode 100644
> > tests/qapi-schema/flat-union-invalid-if-discriminator.exit
> > create mode 100644
> > tests/qapi-schema/flat-union-invalid-if-discriminator.json
> > create mode 100644
> > tests/qapi-schema/flat-union-invalid-if-discriminator.out
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 9b95f8cfe9..13fbb28493 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -577,7 +577,8 @@ def find_alternate_member_qtype(qapi_type):
> >
> > # Return the discriminator enum define if discriminator is specified as an
> > # enum type, otherwise return None.
> > -def discriminator_find_enum_define(expr):
> > +def discriminator_find_enum_define(expr, info):
> > +name = expr['union']
> > base = expr.get('base')
> > discriminator = expr.get('discriminator')
> >
> > @@ -592,6 +593,11 @@ def discriminator_find_enum_define(expr):
> > if not discriminator_member:
> > return None
> >
> > +if discriminator_member.get('if'):
> > +raise QAPISemError(info, 'The discriminator %s.%s for union %s '
> > + 'must not be conditional' %
> > + (base, discriminator, name))
> > +
> > return enum_types.get(discriminator_member['type'])
> >
> >
>
> I'm afraid this isn't the proper place to check. It's an accessor
> function for @struct_types and @enum_types. The check should go into
> check_union(), like this:
indeed, thanks for the hint
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 0cf39df404..c1bc9e9c29 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -799,6 +794,11 @@ def check_union(expr, info):
> "Discriminator '%s' is not a member of base "
> "struct '%s'"
> % (discriminator, base))
> +if discriminator_member.get('if'):
> +raise QAPISemError(info, 'The discriminator %s.%s for union %s '
> + 'must not be conditional' %
> + (base, discriminator, name))
> +
> enum_define = enum_types.get(discriminator_member['type'])
> allow_metas = ['struct']
> # Do not allow string discriminator
>
> > @@ -1020,7 +1026,8 @@ def check_exprs(exprs):
> >
> > if 'include' in expr:
> > continue
> > -if 'union' in expr and not discriminator_find_enum_define(expr):
> > +info = expr_elem['info']
> > +if 'union' in expr and not discriminator_find_enum_define(expr,
> > info):
> > name = '%sKind' % expr['union']
> > elif 'alternate' in expr:
> > name = '%sKind' % expr['alternate']
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 43e100a6cd..abc3fdf764 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -510,6 +510,7 @@ qapi-schema += flat-union-inline.json
> > qapi-schema += flat-union-int-branch.json
> > qapi-schema += flat-union-invalid-branch-key.json
> > qapi-schema += flat-union-invalid-discriminator.json
> > +qapi-schema += flat-union-invalid-if-discriminator.json
> > qapi-schema += flat-union-no-base.json
> > qapi-schema += flat-union-optional-discriminator.json
> > qapi-schema += flat-union-string-discriminator.json
> > diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.err
> > b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
> > new file mode 100644
> > index 00..0c94c9860d
> > --- /dev/null
> > +++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/flat-union-invalid-if-discriminator.json:13: The
> > discriminator TestBase.enum1 for union TestUnion must not be conditional
> > diff --git
Re: [Qemu-devel] [PATCH v6 21/27] qapi: add #if conditions to generated code members
On Mon, Dec 10, 2018 at 2:11 PM Markus Armbruster wrote: > > Markus Armbruster writes: > > > Marc-André Lureau writes: > > > >> Hi > >> On Thu, Dec 6, 2018 at 9:42 PM Markus Armbruster wrote: > >>> > >>> Marc-André Lureau writes: > >>> > >>> > Wrap generated enum/struct members and code with #if/#endif, using the > >>> > >>> enum and struct members > >> > >> ok > >> > >>> > >>> > .ifcond members added in the previous patches. > >>> > > >>> > Some types generate both enum and struct members for example, so a > >>> > step-by-step is unnecessarily complicated to deal with (it would > >>> > easily generate invalid intermediary code). > >>> > >>> Can you give an example of a schema definition that would lead to > >>> complications? > >>> > >> > >> Honestly, I don't remember well (it's been a while I wrote that code). > > > > I know... > > > >> It must be related to implicit enums, such as union kind... If there > >> is no strong need to split this patch, I would rather not do that > >> extra work. > > > > I'm not looking for reasons to split this patch, I'm looking for > > stronger reasons to keep it just like it is :) > > > > Your hunch that complications would arise for simple unions plausible: > > there the same conditional needs to be applied both to the C enum's > > member and the C union member. > > > > For the generated C code to compile, each union tag enum member > > conditional must imply the corresponding variant conditional. > > > > For flat unions, the two are separate. The QAPI generator makes no > > effort to check the enum member's if condition implies the union > > variant's if condition; if you mess them up in the schema, you get to > > deal with the C compilation errors. > > > > For simple unions, the two are one. > > > > If we separate the generator updates for enums and for union members, > > and do enum members first, then unions with conditional tag members > > can't compile. Corrollary: simple unions with conditional variants > > can't compile. > > > > What if we do union members first? > > > > Again, I'm not asking for patch splitting here, I'm just trying to > > arrive at a clearer understanding to avoid making insufficiently > > supported claims in the commit message. The combined patch looks small > > and clean enough to keep it combined. > > > > [...] > > What about this commit message: > > qapi: Add #if conditions to generated code members > > Wrap generated enum and struct members and their supporting code with > #if/#endif, using the .ifcond members added in the previous patches. > > We do enum and struct in a single patch because union tag enum and the > associated variants tie them together, and dealing with that to split > the patch doesn't seem worthwhile. ack, thanks
Re: [Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook
On Mon, 10 Dec 2018 at 18:31, Peter Maydell wrote:
>
> On Mon, 10 Dec 2018 at 17:57, Peter Maydell wrote:
> >
> > Switch the microblaze target from the old unassigned_access hook
> > to the transaction_failed hook.
> >
> > The notable difference is that rather than it being called
> > for all physical memory accesses which fail (including
> > those made by DMA devices or by the gdbstub), it is only
> > called for those made by the CPU via its MMU. For
> > microblaze this makes no difference because none of the
> > target CPU code needs to make loads or stores by physical
> > address.
> >
> > Signed-off-by: Peter Maydell
> > ---
> > A straightforward conversion, but tagged RFC because I don't have
> > any microblaze test images and have tested only with "make check".
> >
> > target/microblaze/cpu.h | 7 ---
> > target/microblaze/cpu.c | 2 +-
> > target/microblaze/op_helper.c | 20 ++--
> > 3 files changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> > index 3c4e0ba80ac..03ca91007d5 100644
> > --- a/target/microblaze/cpu.h
> > +++ b/target/microblaze/cpu.h
> > @@ -388,9 +388,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState
> > *env, target_ulong *pc,
> > }
> >
> > #if !defined(CONFIG_USER_ONLY)
> > -void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> > - bool is_write, bool is_exec, int is_asi,
> > - unsigned size);
> > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > + unsigned size, MMUAccessType access_type,
> > + int mmu_idx, MemTxAttrs attrs,
> > + MemTxResult response, uintptr_t retaddr);
> > #endif
> >
> > #endif
> > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > index 9b546a2c18e..49876b19b38 100644
> > --- a/target/microblaze/cpu.c
> > +++ b/target/microblaze/cpu.c
> > @@ -297,7 +297,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void
> > *data)
> > #ifdef CONFIG_USER_ONLY
> > cc->handle_mmu_fault = mb_cpu_handle_mmu_fault;
> > #else
> > -cc->do_unassigned_access = mb_cpu_unassigned_access;
> > +cc->do_transaction_failed = mb_cpu_transaction_failed;
> > cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
> > #endif
> > dc->vmsd = _mb_cpu;
> > diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> > index 7cdbbcccaef..d25d3b626c8 100644
> > --- a/target/microblaze/op_helper.c
> > +++ b/target/microblaze/op_helper.c
> > @@ -486,18 +486,18 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext,
> > uint32_t rn, uint32_t v)
> > mmu_write(env, ext, rn, v);
> > }
> >
> > -void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> > - bool is_write, bool is_exec, int is_asi,
> > - unsigned size)
> > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > + unsigned size, MMUAccessType access_type,
> > + int mmu_idx, MemTxAttrs attrs,
> > + MemTxResult response, uintptr_t retaddr)
> > {
> > MicroBlazeCPU *cpu;
> > CPUMBState *env;
> > -
> > -qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d
> > exe=%d\n",
> > - addr, is_write ? 1 : 0, is_exec ? 1 : 0);
> > -if (cs == NULL) {
> > -return;
> > -}
> > +qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> > + " physaddr 0x" TARGET_FMT_plx "size %d access type %s\n",
> > + addr, physaddr, size,
> > + access_type == MMU_INST_FETCH ? "INST_FETCH" :
> > + (access_type == MMU_DATA_LOAD ? "DATA_LOAD" :
> > "DATA_STORE"));
> > cpu = MICROBLAZE_CPU(cs);
> > env = >env;
Oops. Here there should be
cpu_restore_state(cs, retaddr, true);
> > if (!(env->sregs[SR_MSR] & MSR_EE)) {
> > @@ -505,7 +505,7 @@ void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> > }
> >
> > env->sregs[SR_EAR] = addr;
> > -if (is_exec) {
> > +if (access_type == MMU_INST_FETCH) {
> > if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
> > env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
> > helper_raise_exception(env, EXCP_HW_EXCP);
> > --
> > 2.19.2
thanks
-- PMM
Re: [Qemu-devel] [PATCH for-3.2 01/11] vhost-user: define conventions for vhost-user backends
Hi On Mon, Dec 10, 2018 at 6:30 PM Gerd Hoffmann wrote: > > On Mon, Nov 26, 2018 at 04:42:40PM +0400, Marc-André Lureau wrote: > > As discussed during "[PATCH v4 00/29] vhost-user for input & GPU" > > review, let's define a common set of backend conventions to help with > > management layer implementation, and interoperability. > > > > Signed-off-by: Marc-André Lureau > > Reviewed-by: Daniel P. Berrangé > > Acked-by: Gerd Hoffmann > > btw: have you seen the idea to use a vfio-style interface for > communication between qemu and external device emulation processes? I heard there was some discussion during KVM forum. Fwiwi, this is also an idea I proposed last year (and quickly discussed during my talk about multi-process qemu). I also experimented with the idea, and wrote a vfio-user backend, with a PCI serial device running in a seperate process: the qemu tree: https://github.com/elmarco/qemu/tree/wip/vfio-user (dirty tree) and the serial device: https://github.com/elmarco/qemu/blob/wip/vfio-user/contrib/libvfio-user/vfio-user-serial.c > > cheers, > Gerd >
Re: [Qemu-devel] [RFC PATCH 2/6] target/ppc: introduce get_avr64() and set_avr64() helpers for VMX register access
On 12/7/18 2:56 AM, Mark Cave-Ayland wrote:
> +avr = tcg_temp_new_i64();
> \
> EA = tcg_temp_new();
> \
> gen_addr_reg_index(ctx, EA);
> \
> tcg_gen_andi_tl(EA, EA, ~0xf);
> \
> /* We only need to swap high and low halves. gen_qemu_ld64_i64 does
> \
> necessary 64-bit byteswap already. */
> \
> if (ctx->le_mode) {
> \
> -gen_qemu_ld64_i64(ctx, cpu_avrl[rD(ctx->opcode)], EA);
> \
> +gen_qemu_ld64_i64(ctx, avr, EA);
> \
> +set_avr64(rD(ctx->opcode), avr, false);
> \
> tcg_gen_addi_tl(EA, EA, 8);
> \
> -gen_qemu_ld64_i64(ctx, cpu_avrh[rD(ctx->opcode)], EA);
> \
> +gen_qemu_ld64_i64(ctx, avr, EA);
> \
> +set_avr64(rD(ctx->opcode), avr, true);
An accurate conversion, but I'm going to call this an existing bug:
The writeback to both avr{l,h} should be delayed until all exceptions have been
raised. Thus you should perform the two gen_qemu_ld64_i64 into two temporaries
and only then write them both back via set_avr64.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [Qemu-devel] [RFC PATCH 1/6] target/ppc: introduce get_fpr() and set_fpr() helpers for FP register access
On 12/7/18 2:56 AM, Mark Cave-Ayland wrote:
> -gen_helper_f##op(cpu_fpr[rD(ctx->opcode)], cpu_env,
> \
> - cpu_fpr[rA(ctx->opcode)],
> \
> - cpu_fpr[rC(ctx->opcode)], cpu_fpr[rB(ctx->opcode)]);
> \
> +get_fpr(t0, rA(ctx->opcode));
> \
> +get_fpr(t1, rC(ctx->opcode));
> \
> +get_fpr(t2, rB(ctx->opcode));
> \
> +gen_helper_f##op(t3, cpu_env, t0, t1, t2);
> \
> +set_fpr(rD(ctx->opcode), t3);
> \
> if (isfloat) {
> \
> -gen_helper_frsp(cpu_fpr[rD(ctx->opcode)], cpu_env,
> \
> -cpu_fpr[rD(ctx->opcode)]);
> \
> +get_fpr(t0, rD(ctx->opcode));
> \
> +gen_helper_frsp(t3, cpu_env, t0);
> \
> +set_fpr(rD(ctx->opcode), t3);
> \
> }
> \
This is an accurate conversion, but the writeback to the rD register need not
happen until after helper_frsp. Just move it below the isfloat block.
I do see that helper_frsp can raise an exception for invalid_op for SNaN. If
that code were actually reachable, this would have been an existing bug, in
that we should not have written back to rD after the first operation. However,
any SNaN will already have been eliminated by the first operation (via
squashing to QNaN or by exiting via exception).
Similarly in GEN_FLOAT_AB.
> +get_fpr(t0, rB(ctx->opcode));
> +gen_helper_frsqrte(t1, cpu_env, t0);
> +set_fpr(rD(ctx->opcode), t1);
> +gen_helper_frsp(t1, cpu_env, t1);
> +gen_compute_fprf_float64(t1);
gen_frsqrtes has the set_fpr in the wrong place. Likewise gen_fsqrts.
r~
Re: [Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook
On Mon, 10 Dec 2018 at 17:57, Peter Maydell wrote:
>
> Switch the microblaze target from the old unassigned_access hook
> to the transaction_failed hook.
>
> The notable difference is that rather than it being called
> for all physical memory accesses which fail (including
> those made by DMA devices or by the gdbstub), it is only
> called for those made by the CPU via its MMU. For
> microblaze this makes no difference because none of the
> target CPU code needs to make loads or stores by physical
> address.
>
> Signed-off-by: Peter Maydell
> ---
> A straightforward conversion, but tagged RFC because I don't have
> any microblaze test images and have tested only with "make check".
>
> target/microblaze/cpu.h | 7 ---
> target/microblaze/cpu.c | 2 +-
> target/microblaze/op_helper.c | 20 ++--
> 3 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index 3c4e0ba80ac..03ca91007d5 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -388,9 +388,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState *env,
> target_ulong *pc,
> }
>
> #if !defined(CONFIG_USER_ONLY)
> -void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> - bool is_write, bool is_exec, int is_asi,
> - unsigned size);
> +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> + unsigned size, MMUAccessType access_type,
> + int mmu_idx, MemTxAttrs attrs,
> + MemTxResult response, uintptr_t retaddr);
> #endif
>
> #endif
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 9b546a2c18e..49876b19b38 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -297,7 +297,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
> #ifdef CONFIG_USER_ONLY
> cc->handle_mmu_fault = mb_cpu_handle_mmu_fault;
> #else
> -cc->do_unassigned_access = mb_cpu_unassigned_access;
> +cc->do_transaction_failed = mb_cpu_transaction_failed;
> cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
> #endif
> dc->vmsd = _mb_cpu;
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 7cdbbcccaef..d25d3b626c8 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -486,18 +486,18 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext,
> uint32_t rn, uint32_t v)
> mmu_write(env, ext, rn, v);
> }
>
> -void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> - bool is_write, bool is_exec, int is_asi,
> - unsigned size)
> +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> + unsigned size, MMUAccessType access_type,
> + int mmu_idx, MemTxAttrs attrs,
> + MemTxResult response, uintptr_t retaddr)
> {
> MicroBlazeCPU *cpu;
> CPUMBState *env;
> -
> -qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d
> exe=%d\n",
> - addr, is_write ? 1 : 0, is_exec ? 1 : 0);
> -if (cs == NULL) {
> -return;
> -}
> +qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> + " physaddr 0x" TARGET_FMT_plx "size %d access type %s\n",
> + addr, physaddr, size,
> + access_type == MMU_INST_FETCH ? "INST_FETCH" :
> + (access_type == MMU_DATA_LOAD ? "DATA_LOAD" :
> "DATA_STORE"));
> cpu = MICROBLAZE_CPU(cs);
> env = >env;
> if (!(env->sregs[SR_MSR] & MSR_EE)) {
> @@ -505,7 +505,7 @@ void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> }
>
> env->sregs[SR_EAR] = addr;
> -if (is_exec) {
> +if (access_type == MMU_INST_FETCH) {
> if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
> env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
> helper_raise_exception(env, EXCP_HW_EXCP);
> --
> 2.19.2
>
>
--
12345678901234567890123456789012345678901234567890123456789012345678901234567890
1 2 3 4 5 6 7 8
[Qemu-devel] [PATCH 8/8] checkpatch: warn about qemu/queue.h head structs that are not typedef-ed
These are just like any other struct or union, so they should have
CamelCase typedefs.
Signed-off-by: Paolo Bonzini
---
scripts/checkpatch.pl | 5 +
1 file changed, 5 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a8d6e44107..b4b3495044 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2263,6 +2263,11 @@ sub process {
}
}
+ if ($line =~
/^.\s*(Q(?:S?LIST|SIMPLEQ|TAILQ)_HEAD)\s*\(\s*[^,]/ &&
+ $line !~ /^.typedef/) {
+ ERROR("named $1 should be typedefed separately\n" .
$herecurr);
+ }
+
# Need a space before open parenthesis after if, while etc
if ($line=~/\b(if|while|for|switch)\(/) {
ERROR("space required before the open parenthesis
'('\n" . $herecurr);
--
2.19.2
[Qemu-devel] [PATCH 6/8] qemu/queue.h: reimplement QTAILQ without pointer-to-pointers
QTAILQ is a doubly linked list, with a pointer-to-pointer to the last
element from the head, and the previous element from each node.
But if you squint enough, QTAILQ becomes a combination of a singly-linked
forwards list, and another singly-linked list which goes backwards and
is circular. This is the idea that lets QTAILQ implement reverse
iteration: only, because the backwards list points inside the node,
accessing the previous element needs to go two steps back and one
forwards.
What this patch does is implement it in these terms, without actually
changing the in-memory layout at all. The coexistence of the two lists
is realized by making QTAILQ_HEAD and QTAILQ_ENTRY unions of the forwards
pointer and a generic QTailQLink node. Thq QTailQLink can walk the list in
both directions; the union is needed so that the forwards pointer can
have the correct type, as a sort of poor man's template. While there
are other ways to get the same layout without a union, this one has
the advantage of simpler operation in the debugger, because the fields
tqh_first and tqe_next still exist as before the patch. Those fields are
also used by scripts/qemugdb/mtree.py, so it's a good idea to preserve them.
The advantage of the new representation is that the two-back-one-forward
dance done by backwards accesses can be done all while operating on
QTailQLinks. No casting to the head struct is needed anymore because,
even though the QTailQLink's forward pointer is a void *, we can use
typeof to recover the correct type. This patch only changes the
implementation, not the interface. The next patch will remove the head
struct name from the backwards visit macros.
Signed-off-by: Paolo Bonzini
---
include/qemu/queue.h | 139 -
include/qemu/rcu_queue.h | 45 ++--
scripts/cocci-macro-file.h | 14 ++--
3 files changed, 92 insertions(+), 106 deletions(-)
diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index b9571e93d8..a893facb86 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -346,23 +346,28 @@ struct {
\
#define QSIMPLEQ_FIRST(head)((head)->sqh_first)
#define QSIMPLEQ_NEXT(elm, field) ((elm)->field.sqe_next)
+typedef struct QTailQLink {
+void *tql_next;
+struct QTailQLink *tql_prev;
+} QTailQLink;
/*
- * Tail queue definitions.
+ * Tail queue definitions. The union acts as a poor man template, as if
+ * it were QTailQLink.
*/
#define QTAILQ_HEAD(name, type) \
-struct name { \
-type *tqh_first; /* first element */ \
-type **tqh_last; /* addr of last next element */ \
+union name {\
+struct type *tqh_first; /* first element */ \
+QTailQLink tqh_circ; /* link for circular backwards list */ \
}
#define QTAILQ_HEAD_INITIALIZER(head) \
-{ NULL, &(head).tqh_first }
+{ .tqh_circ = { NULL, &(head).tqh_circ } }
#define QTAILQ_ENTRY(type) \
-struct {\
-type *tqe_next; /* next element */ \
-type **tqe_prev; /* address of previous next element */\
+union { \
+struct type *tqe_next;/* next element */\
+QTailQLink tqe_circ; /* link for circular backwards list */ \
}
/*
@@ -370,51 +375,51 @@ struct {
\
*/
#define QTAILQ_INIT(head) do { \
(head)->tqh_first = NULL; \
-(head)->tqh_last = &(head)->tqh_first; \
+(head)->tqh_circ.tql_prev = &(head)->tqh_circ; \
} while (/*CONSTCOND*/0)
#define QTAILQ_INSERT_HEAD(head, elm, field) do { \
if (((elm)->field.tqe_next = (head)->tqh_first) != NULL)\
-(head)->tqh_first->field.tqe_prev = \
-&(elm)->field.tqe_next; \
+(head)->tqh_first->field.tqe_circ.tql_prev =\
+&(elm)->field.tqe_circ; \
else\
-(head)->tqh_last = &(elm)->field.tqe_next; \
+(head)->tqh_circ.tql_prev = &(elm)->field.tqe_circ; \
(head)->tqh_first = (elm); \
-(elm)->field.tqe_prev = &(head)->tqh_first;
[Qemu-devel] [PATCH 4/8] qemu/queue.h: typedef QTAILQ heads
This will be needed when we change the QTAILQ head and elem structs
to unions. However, it is also consistent with the usage elsewhere
in QEMU for other list head structs (see for example FsMountList).
Note that most QTAILQs only need their name in order to do backwards
walks. Those do not break with the struct->union change, and anyway
the change will also remove the need to name heads when doing backwards
walks, so those are not touched here.
Signed-off-by: Paolo Bonzini
---
exec.c| 3 ++-
hw/vfio/common.c | 2 +-
include/hw/vfio/vfio-common.h | 3 ++-
include/qom/cpu.h | 5 +++--
ui/input.c| 14 --
5 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/exec.c b/exec.c
index b6b2007f27..a629c98eb5 100644
--- a/exec.c
+++ b/exec.c
@@ -94,7 +94,8 @@ int target_page_bits;
bool target_page_bits_decided;
#endif
-struct CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
+CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
+
/* current CPU in the current thread. It is only valid inside
cpu_exec() */
__thread CPUState *current_cpu;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7aa804ea0b..4262b80c44 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -37,7 +37,7 @@
#include "trace.h"
#include "qapi/error.h"
-struct vfio_group_head vfio_group_list =
+VFIOGroupList vfio_group_list =
QLIST_HEAD_INITIALIZER(vfio_group_list);
static QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces =
QLIST_HEAD_INITIALIZER(vfio_address_spaces);
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 127ca47815..7624c9f511 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -180,7 +180,8 @@ int vfio_get_device(VFIOGroup *group, const char *name,
VFIODevice *vbasedev, Error **errp);
extern const MemoryRegionOps vfio_region_ops;
-extern QLIST_HEAD(vfio_group_head, VFIOGroup) vfio_group_list;
+typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
+extern VFIOGroupList vfio_group_list;
#ifdef CONFIG_LINUX
int vfio_get_region_info(VFIODevice *vbasedev, int index,
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 62aef77b87..4662a205c1 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -435,8 +435,9 @@ struct CPUState {
GArray *iommu_notifiers;
};
-QTAILQ_HEAD(CPUTailQ, CPUState);
-extern struct CPUTailQ cpus;
+typedef QTAILQ_HEAD(CPUTailQ, CPUState) CPUTailQ;
+extern CPUTailQ cpus;
+
#define first_cpuQTAILQ_FIRST_RCU()
#define CPU_NEXT(cpu)QTAILQ_NEXT_RCU(cpu, node)
#define CPU_FOREACH(cpu) QTAILQ_FOREACH_RCU(cpu, , node)
diff --git a/ui/input.c b/ui/input.c
index 7c9a4109c4..35c7964f64 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -19,6 +19,9 @@ struct QemuInputHandlerState {
};
typedef struct QemuInputEventQueue QemuInputEventQueue;
+typedef QTAILQ_HEAD(QemuInputEventQueueHead, QemuInputEventQueue)
+QemuInputEventQueueHead;
+
struct QemuInputEventQueue {
enum {
QEMU_INPUT_QUEUE_DELAY = 1,
@@ -37,8 +40,7 @@ static QTAILQ_HEAD(, QemuInputHandlerState) handlers =
static NotifierList mouse_mode_notifiers =
NOTIFIER_LIST_INITIALIZER(mouse_mode_notifiers);
-static QTAILQ_HEAD(QemuInputEventQueueHead, QemuInputEventQueue) kbd_queue =
-QTAILQ_HEAD_INITIALIZER(kbd_queue);
+static QemuInputEventQueueHead kbd_queue = QTAILQ_HEAD_INITIALIZER(kbd_queue);
static QEMUTimer *kbd_timer;
static uint32_t kbd_default_delay_ms = 10;
static uint32_t queue_count;
@@ -257,7 +259,7 @@ static void qemu_input_event_trace(QemuConsole *src,
InputEvent *evt)
static void qemu_input_queue_process(void *opaque)
{
-struct QemuInputEventQueueHead *queue = opaque;
+QemuInputEventQueueHead *queue = opaque;
QemuInputEventQueue *item;
g_assert(!QTAILQ_EMPTY(queue));
@@ -288,7 +290,7 @@ static void qemu_input_queue_process(void *opaque)
}
}
-static void qemu_input_queue_delay(struct QemuInputEventQueueHead *queue,
+static void qemu_input_queue_delay(QemuInputEventQueueHead *queue,
QEMUTimer *timer, uint32_t delay_ms)
{
QemuInputEventQueue *item = g_new0(QemuInputEventQueue, 1);
@@ -306,7 +308,7 @@ static void qemu_input_queue_delay(struct
QemuInputEventQueueHead *queue,
}
}
-static void qemu_input_queue_event(struct QemuInputEventQueueHead *queue,
+static void qemu_input_queue_event(QemuInputEventQueueHead *queue,
QemuConsole *src, InputEvent *evt)
{
QemuInputEventQueue *item = g_new0(QemuInputEventQueue, 1);
@@ -318,7 +320,7 @@ static void qemu_input_queue_event(struct
QemuInputEventQueueHead *queue,
queue_count++;
}
-static void qemu_input_queue_sync(struct QemuInputEventQueueHead *queue)
+static void qemu_input_queue_sync(QemuInputEventQueueHead *queue)
{
QemuInputEventQueue *item = g_new0(QemuInputEventQueue, 1);
--
2.19.2
[Qemu-devel] [PATCH 3/8] qemu/queue.h: leave head structs anonymous unless necessary
Most list head structs need not be given a name. In most cases the
name is given just in case one is going to use QTAILQ_LAST, QTAILQ_PREV
or reverse iteration, but this does not apply to lists of other kinds,
and even for QTAILQ in practice this is only rarely needed. In addition,
we will soon reimplement those macros completely so that they do not
need a name for the head struct. So clean up everything, not giving a
name except in the rare case where it is necessary.
Signed-off-by: Paolo Bonzini
---
accel/kvm/kvm-all.c | 4 ++--
block/gluster.c | 2 +-
block/mirror.c | 2 +-
block/qcow2-bitmap.c| 4 +---
block/qcow2.h | 2 +-
block/sheepdog.c| 6 +++---
block/vhdx.h| 2 +-
blockdev.c | 2 +-
contrib/ivshmem-client/ivshmem-client.h | 4 +---
contrib/ivshmem-server/ivshmem-server.h | 5 +
exec.c | 2 +-
fsdev/qemu-fsdev.c | 2 +-
hw/block/nvme.h | 8
hw/block/xen_disk.c | 6 +++---
hw/core/reset.c | 2 +-
hw/i386/xen/xen-mapcache.c | 2 +-
hw/ppc/spapr_iommu.c| 2 +-
hw/usb/ccid-card-emulated.c | 4 ++--
hw/usb/dev-network.c| 2 +-
hw/usb/xen-usb.c| 6 +++---
hw/watchdog/watchdog.c | 2 +-
hw/xen/xen_pvdev.c | 4 ++--
include/exec/memory.h | 4 ++--
include/hw/vfio/vfio-platform.h | 2 +-
include/qom/cpu.h | 4 ++--
include/sysemu/kvm.h| 2 --
include/sysemu/rng.h| 2 +-
linux-user/elfload.c| 2 +-
memory.c| 2 +-
migration/block-dirty-bitmap.c | 2 +-
migration/block.c | 4 ++--
migration/ram.c | 2 +-
monitor.c | 4 ++--
net/queue.c | 2 +-
net/slirp.c | 2 +-
slirp/slirp.c | 2 +-
target/arm/kvm.c| 2 +-
target/i386/hax-mem.c | 2 +-
tcg/tcg.h | 2 +-
tests/test-rcu-list.c | 2 +-
vl.c| 2 +-
41 files changed, 56 insertions(+), 65 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 4880a05399..4e1de942ce 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -86,7 +86,7 @@ struct KVMState
int robust_singlestep;
int debugregs;
#ifdef KVM_CAP_SET_GUEST_DEBUG
-struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
+QTAILQ_HEAD(, kvm_sw_breakpoint) kvm_sw_breakpoints;
#endif
int many_ioeventfds;
int intx_set_mask;
@@ -102,7 +102,7 @@ struct KVMState
int nr_allocated_irq_routes;
unsigned long *used_gsi_bitmap;
unsigned int gsi_count;
-QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
+QTAILQ_HEAD(, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
#endif
KVMMemoryListener memory_listener;
QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
diff --git a/block/gluster.c b/block/gluster.c
index 5e300c96c8..72891060e3 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -72,7 +72,7 @@ typedef struct ListElement {
GlfsPreopened saved;
} ListElement;
-static QLIST_HEAD(glfs_list, ListElement) glfs_list;
+static QLIST_HEAD(, ListElement) glfs_list;
static QemuOptsList qemu_gluster_create_opts = {
.name = "qemu-gluster-create-opts",
diff --git a/block/mirror.c b/block/mirror.c
index 8f52c6215d..6250cc3c87 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -72,7 +72,7 @@ typedef struct MirrorBlockJob {
unsigned long *in_flight_bitmap;
int in_flight;
int64_t bytes_in_flight;
-QTAILQ_HEAD(MirrorOpList, MirrorOp) ops_in_flight;
+QTAILQ_HEAD(, MirrorOp) ops_in_flight;
int ret;
bool unmap;
int target_cluster_size;
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index accebef4cf..b946301429 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -77,8 +77,6 @@ typedef struct Qcow2BitmapTable {
uint32_t size; /* number of 64bit entries */
QSIMPLEQ_ENTRY(Qcow2BitmapTable) entry;
} Qcow2BitmapTable;
-typedef QSIMPLEQ_HEAD(Qcow2BitmapTableList, Qcow2BitmapTable)
-Qcow2BitmapTableList;
typedef struct Qcow2Bitmap {
Qcow2BitmapTable table;
@@ -1316,7 +1314,7 @@ void
qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
int ret;
Qcow2BitmapList *bm_list;
Qcow2Bitmap *bm;
-Qcow2BitmapTableList drop_tables;
+QSIMPLEQ_HEAD(, Qcow2BitmapTable) drop_tables;
Qcow2BitmapTable *tb, *tb_next;
if
[Qemu-devel] [PATCH 5/8] qemu/queue.h: remove Q_TAILQ_{HEAD, ENTRY}
These are not present for other kinds of queue, and unused.
Zap them before more changes are made to the QTAILQ
implementation.
Signed-off-by: Paolo Bonzini
---
include/qemu/queue.h | 14 ++
scripts/cocci-macro-file.h | 10 --
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index ac418efc43..b9571e93d8 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -350,22 +350,20 @@ struct {
\
/*
* Tail queue definitions.
*/
-#define Q_TAILQ_HEAD(name, type, qual) \
+#define QTAILQ_HEAD(name, type) \
struct name { \
-qual type *tqh_first; /* first element */ \
-qual type *qual *tqh_last; /* addr of last next element */ \
+type *tqh_first; /* first element */ \
+type **tqh_last; /* addr of last next element */ \
}
-#define QTAILQ_HEAD(name, type) Q_TAILQ_HEAD(name, struct type,)
#define QTAILQ_HEAD_INITIALIZER(head) \
{ NULL, &(head).tqh_first }
-#define Q_TAILQ_ENTRY(type, qual) \
+#define QTAILQ_ENTRY(type) \
struct {\
-qual type *tqe_next;/* next element */ \
-qual type *qual *tqe_prev; /* address of previous next element */\
+type *tqe_next; /* next element */ \
+type **tqe_prev; /* address of previous next element */\
}
-#define QTAILQ_ENTRY(type) Q_TAILQ_ENTRY(struct type,)
/*
* Tail queue functions.
diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
index 9f2e72e7e1..5c49369dac 100644
--- a/scripts/cocci-macro-file.h
+++ b/scripts/cocci-macro-file.h
@@ -93,11 +93,6 @@ struct {
\
/*
* Tail queue definitions.
*/
-#define Q_TAILQ_HEAD(name, type, qual) \
-struct name { \
-qual type *tqh_first; /* first element */ \
-qual type *qual *tqh_last; /* addr of last next element */ \
-}
#define QTAILQ_HEAD(name, type) \
struct name { \
type *tqh_first; /* first element */ \
@@ -107,11 +102,6 @@ struct name {
\
#define QTAILQ_HEAD_INITIALIZER(head) \
{ NULL, &(head).tqh_first }
-#define Q_TAILQ_ENTRY(type, qual) \
-struct {\
-qual type *tqe_next;/* next element */ \
-qual type *qual *tqe_prev; /* address of previous next element */\
-}
#define QTAILQ_ENTRY(type) \
struct {\
type *tqe_next; /* next element */\
--
2.19.2
[Qemu-devel] [PATCH 1/8] qemu/queue.h: do not access tqe_prev directly
Use the QTAILQ_IN_USE macro instead, it does the same thing but in a
few patches its definition will change.
Signed-off-by: Paolo Bonzini
---
blockdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 81f95d920b..7604b2183b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4259,7 +4259,7 @@ void qmp_blockdev_del(const char *node_name, Error **errp)
goto out;
}
-if (!bs->monitor_list.tqe_prev) {
+if (!QTAILQ_IN_USE(bs, monitor_list)) {
error_setg(errp, "Node %s is not owned by the monitor",
bs->node_name);
goto out;
--
2.19.2
[Qemu-devel] [PATCH 2/8] vfio: make vfio_address_spaces static
It is not used outside hw/vfio/common.c, so it does not need to be extern. Cc: Alex Williamson Signed-off-by: Paolo Bonzini --- hw/vfio/common.c | 2 +- include/hw/vfio/vfio-common.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 7c185e5a2e..7aa804ea0b 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -39,7 +39,7 @@ struct vfio_group_head vfio_group_list = QLIST_HEAD_INITIALIZER(vfio_group_list); -struct vfio_as_head vfio_address_spaces = +static QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces = QLIST_HEAD_INITIALIZER(vfio_address_spaces); #ifdef CONFIG_KVM diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index 1b434d02f6..127ca47815 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -181,7 +181,6 @@ int vfio_get_device(VFIOGroup *group, const char *name, extern const MemoryRegionOps vfio_region_ops; extern QLIST_HEAD(vfio_group_head, VFIOGroup) vfio_group_list; -extern QLIST_HEAD(vfio_as_head, VFIOAddressSpace) vfio_address_spaces; #ifdef CONFIG_LINUX int vfio_get_region_info(VFIODevice *vbasedev, int index, -- 2.19.2
[Qemu-devel] [PATCH for-4.0 v2 0/8] qemu/queue.h usage cleanup, improved QTAILQ API
This series includes two changes that are a bit intertwined.
The main one is to reimplement QTAILQ in a way that simplifies
backwards walking of the list. The in-memory layout actually
stays the same, but the C description of it changes so that
(also thanks to typeof) you don't have to specify a name for
the "head" struct in QTAILQ_{LAST,PREV,FOREACH_REVERSE}.
This is done in patches 1, 4 and 5.
Once you do this, you actually almost never need to define
a named head struct. Therefore, the series also cleans up
other cases where the struct was given a name unnecessarily,
and ensures that those queue.h structs are given a typedef
and a camel case name, similar to all other structs in QEMU.
This is done in patches 2 and 3, and at the end of the series
we can add a checkpatch test for it in patch 6.
v1->v2: split out patches 2 and 5 [Markus for patch 5]
moved the other VFIO change from patch 3 to 4
Paolo Bonzini (8):
qemu/queue.h: do not access tqe_prev directly
vfio: make vfio_address_spaces static
qemu/queue.h: leave head structs anonymous unless necessary
qemu/queue.h: typedef QTAILQ heads
qemu/queue.h: remove Q_TAILQ_{HEAD,ENTRY}
qemu/queue.h: reimplement QTAILQ without pointer-to-pointers
qemu/queue.h: simplify reverse access to QTAILQ
checkpatch: warn about qemu/queue.h head structs that are not
typedef-ed
accel/kvm/kvm-all.c | 4 +-
block/gluster.c | 2 +-
block/mirror.c | 2 +-
block/qcow2-bitmap.c| 4 +-
block/qcow2.h | 2 +-
block/sheepdog.c| 6 +-
block/vhdx.h| 2 +-
blockdev.c | 4 +-
contrib/ivshmem-client/ivshmem-client.h | 4 +-
contrib/ivshmem-server/ivshmem-server.h | 5 +-
cpus-common.c | 2 +-
dump.c | 2 +-
exec.c | 5 +-
fsdev/qemu-fsdev.c | 2 +-
hw/block/nvme.h | 8 +-
hw/block/xen_disk.c | 6 +-
hw/core/qdev.c | 4 +-
hw/core/reset.c | 2 +-
hw/i386/xen/xen-mapcache.c | 2 +-
hw/ppc/spapr_iommu.c| 2 +-
hw/scsi/scsi-bus.c | 2 +-
hw/usb/ccid-card-emulated.c | 4 +-
hw/usb/combined-packet.c| 2 +-
hw/usb/dev-mtp.c| 4 +-
hw/usb/dev-network.c| 2 +-
hw/usb/hcd-ehci.c | 2 +-
hw/usb/hcd-ehci.h | 2 +-
hw/usb/hcd-uhci.c | 4 +-
hw/usb/xen-usb.c| 6 +-
hw/vfio/common.c| 4 +-
hw/watchdog/watchdog.c | 2 +-
hw/xen/xen_pvdev.c | 4 +-
include/exec/memory.h | 6 +-
include/hw/qdev-core.h | 2 +-
include/hw/usb.h| 2 +-
include/hw/vfio/vfio-common.h | 4 +-
include/hw/vfio/vfio-platform.h | 2 +-
include/net/net.h | 2 +-
include/qemu/option_int.h | 2 +-
include/qemu/queue.h| 153 +++-
include/qemu/rcu_queue.h| 45 +++
include/qom/cpu.h | 9 +-
include/sysemu/kvm.h| 2 -
include/sysemu/memory_mapping.h | 2 +-
include/sysemu/rng.h| 2 +-
linux-user/elfload.c| 2 +-
memory.c| 19 ++-
memory_mapping.c| 2 +-
migration/block-dirty-bitmap.c | 2 +-
migration/block.c | 4 +-
migration/ram.c | 2 +-
monitor.c | 4 +-
net/filter.c| 2 +-
net/net.c | 2 +-
net/queue.c | 2 +-
net/slirp.c | 2 +-
qga/commands-posix.c| 2 +-
scripts/checkpatch.pl | 5 +
scripts/cocci-macro-file.h | 24 ++--
slirp/slirp.c | 2 +-
target/arm/kvm.c| 2 +-
target/i386/hax-mem.c | 2 +-
tcg/tcg.c | 2 +-
tcg/tcg.h | 6 +-
tests/libqos/malloc.c | 2 +-
tests/test-rcu-list.c | 2 +-
tests/test-vmstate.c| 8 +-
ui/console.c| 4 +-
ui/input.c | 14 ++-
util/qemu-option.c | 4 +-
vl.c| 2 +-
71 files changed, 217 insertions(+), 248 deletions(-)
Re: [Qemu-devel] [PATCH 1/9] tests: acpi: remove not used ACPI_READ_GENERIC_ADDRESS macro
On 12/10/18 7:10 PM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov
Reviewed-by: Philippe Mathieu-Daudé
> ---
> tests/acpi-utils.h | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
> index 4f4899d..c8844a2 100644
> --- a/tests/acpi-utils.h
> +++ b/tests/acpi-utils.h
> @@ -70,14 +70,6 @@ typedef struct {
> g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
> } while (0)
>
> -#define ACPI_READ_GENERIC_ADDRESS(field, addr) \
> -do { \
> -ACPI_READ_FIELD((field).space_id, addr); \
> -ACPI_READ_FIELD((field).bit_width, addr);\
> -ACPI_READ_FIELD((field).bit_offset, addr); \
> -ACPI_READ_FIELD((field).access_width, addr); \
> -ACPI_READ_FIELD((field).address, addr); \
> -} while (0)
>
>
> uint8_t acpi_calc_checksum(const uint8_t *data, int len);
>
[Qemu-devel] [PATCH 7/8] qemu/queue.h: simplify reverse access to QTAILQ
The new definition of QTAILQ does not require passing the headname,
remove it.
Signed-off-by: Paolo Bonzini
---
cpus-common.c | 2 +-
dump.c | 2 +-
hw/core/qdev.c | 4 ++--
hw/scsi/scsi-bus.c | 2 +-
hw/usb/combined-packet.c| 2 +-
hw/usb/dev-mtp.c| 4 ++--
hw/usb/hcd-ehci.c | 2 +-
hw/usb/hcd-ehci.h | 2 +-
hw/usb/hcd-uhci.c | 4 ++--
include/exec/memory.h | 2 +-
include/hw/qdev-core.h | 2 +-
include/hw/usb.h| 2 +-
include/net/net.h | 2 +-
include/qemu/option_int.h | 2 +-
include/qemu/queue.h| 16
include/sysemu/memory_mapping.h | 2 +-
memory.c| 17 ++---
memory_mapping.c| 2 +-
net/filter.c| 2 +-
net/net.c | 2 +-
qga/commands-posix.c| 2 +-
tcg/tcg.c | 2 +-
tcg/tcg.h | 4 ++--
tests/libqos/malloc.c | 2 +-
tests/test-vmstate.c| 8
ui/console.c| 4 ++--
util/qemu-option.c | 4 ++--
27 files changed, 48 insertions(+), 53 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 98dd8c6ff1..3ca58c64e8 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -99,7 +99,7 @@ void cpu_list_remove(CPUState *cpu)
return;
}
-assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(, CPUTailQ)));
+assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST()));
QTAILQ_REMOVE_RCU(, cpu, node);
cpu->cpu_index = UNASSIGNED_CPU_INDEX;
diff --git a/dump.c b/dump.c
index 4ec94c5e25..ef1d8025c9 100644
--- a/dump.c
+++ b/dump.c
@@ -1557,7 +1557,7 @@ static void get_max_mapnr(DumpState *s)
{
GuestPhysBlock *last_block;
-last_block = QTAILQ_LAST(>guest_phys_blocks.head, GuestPhysBlockHead);
+last_block = QTAILQ_LAST(>guest_phys_blocks.head);
s->max_mapnr = dump_paddr_to_pfn(s, last_block->target_end);
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27..a7dd4bebd6 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -158,7 +158,7 @@ DeviceState *qdev_try_create(BusState *bus, const char
*type)
return dev;
}
-static QTAILQ_HEAD(device_listeners, DeviceListener) device_listeners
+static QTAILQ_HEAD(, DeviceListener) device_listeners
= QTAILQ_HEAD_INITIALIZER(device_listeners);
enum ListenerDirection { Forward, Reverse };
@@ -177,7 +177,7 @@ enum ListenerDirection { Forward, Reverse };
break;\
case Reverse: \
QTAILQ_FOREACH_REVERSE(_listener, _listeners, \
- device_listeners, link) { \
+ link) {\
if (_listener->_callback) { \
_listener->_callback(_listener, ##_args); \
} \
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 97cd167114..c480553083 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1554,7 +1554,7 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel,
int id, int lun)
BusChild *kid;
SCSIDevice *target_dev = NULL;
-QTAILQ_FOREACH_REVERSE(kid, >qbus.children, ChildrenHead, sibling) {
+QTAILQ_FOREACH_REVERSE(kid, >qbus.children, sibling) {
DeviceState *qdev = kid->child;
SCSIDevice *dev = SCSI_DEVICE(qdev);
diff --git a/hw/usb/combined-packet.c b/hw/usb/combined-packet.c
index 01a7ed0848..fc98383d30 100644
--- a/hw/usb/combined-packet.c
+++ b/hw/usb/combined-packet.c
@@ -64,7 +64,7 @@ void usb_combined_input_packet_complete(USBDevice *dev,
USBPacket *p)
status = combined->first->status;
actual_length = combined->first->actual_length;
-short_not_ok = QTAILQ_LAST(>packets, packets_head)->short_not_ok;
+short_not_ok = QTAILQ_LAST(>packets)->short_not_ok;
QTAILQ_FOREACH_SAFE(p, >packets, combined_entry, next) {
if (!done) {
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 100b7171f4..c33d7d6b1f 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -191,7 +191,7 @@ struct MTPState {
#ifdef CONFIG_INOTIFY1
/* inotify descriptor */
int inotifyfd;
-QTAILQ_HEAD(events, MTPMonEntry) events;
+QTAILQ_HEAD(, MTPMonEntry) events;
#endif
/* Responder is expecting a write operation */
bool write_pending;
@@ -1982,7 +1982,7 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket
*p)
case EP_EVENT:
#ifdef CONFIG_INOTIFY1
if (!QTAILQ_EMPTY(>events)) {
-struct MTPMonEntry *e = QTAILQ_LAST(>events, events);
+struct MTPMonEntry *e = QTAILQ_LAST(>events);
[Qemu-devel] [PATCH 3/9] tests: acpi: make sure FADT is fetched only once
Whole FADT is fetched as part of RSDT referenced tables in
fetch_rsdt_referenced_tables() albeit a bit later than when FADT
is partially parsed in fadt_fetch_facs_and_dsdt_ptrs().
However there is no reason for calling fetch_rsdt_referenced_tables()
so late, just move it right after we fetched RSDT and before
fadt_fetch_facs_and_dsdt_ptrs(). That way we can reuse whole FADT
fetched by fetch_rsdt_referenced_tables() and avoid duplicate
custom fields fetching in fadt_fetch_facs_and_dsdt_ptrs().
While at it rename fadt_fetch_facs_and_dsdt_ptrs() to
test_acpi_fadt_table(). The follow up patch will merge
fadt_fetch_facs_and_dsdt_ptrs() into test_acpi_rsdt_table(),
so that we would end up calling only test_acpi_FOO_table()
for consistency for tables that require special processing.
Signed-off-by: Igor Mammedov
---
tests/bios-tables-test.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 1666cf7..5faf75f 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -139,18 +139,15 @@ static void test_acpi_rsdt_table(test_data *data)
data->rsdt_tables_nr = tables_nr;
}
-static void fadt_fetch_facs_and_dsdt_ptrs(test_data *data)
+static void test_acpi_fadt_table(test_data *data)
{
-uint32_t addr;
-AcpiTableHeader hdr;
+/* FADT table is 1st */
+AcpiSdtTable *fadt = _array_index(data->tables, typeof(*fadt), 0);
-/* FADT table comes first */
-addr = le32_to_cpu(data->rsdt_tables_addr[0]);
-ACPI_READ_TABLE_HEADER(, addr);
-ACPI_ASSERT_CMP(hdr.signature, "FACP");
+ACPI_ASSERT_CMP(fadt->header->signature, "FACP");
-ACPI_READ_FIELD(data->facs_addr, addr);
-ACPI_READ_FIELD(data->dsdt_addr, addr);
+memcpy(>facs_addr, fadt->aml + 36 /* FIRMWARE_CTRL */, 4);
+memcpy(>dsdt_addr, fadt->aml + 40 /* DSDT */, 4);
}
static void sanitize_fadt_ptrs(test_data *data)
@@ -622,10 +619,10 @@ static void test_acpi_one(const char *params, test_data
*data)
test_acpi_rsdp_address(data);
test_acpi_rsdp_table(data);
test_acpi_rsdt_table(data);
-fadt_fetch_facs_and_dsdt_ptrs(data);
+fetch_rsdt_referenced_tables(data);
+test_acpi_fadt_table(data);
test_acpi_facs_table(data);
test_acpi_dsdt_table(data);
-fetch_rsdt_referenced_tables(data);
sanitize_fadt_ptrs(data);
--
2.7.4
Re: [Qemu-devel] [RFC PATCH 1/6] target/ppc: introduce get_fpr() and set_fpr() helpers for FP register access
On 12/9/18 11:17 PM, David Gibson wrote: > On Fri, Dec 07, 2018 at 08:56:30AM +, Mark Cave-Ayland wrote: >> These helpers allow us to move FP register values to/from the specified >> TCGv_i64 >> argument. >> >> To prevent FP helpers accessing the cpu_fpr array directly, add extra TCG >> temporaries as required. > > It's not obvious to me why that's a desirable thing. I'm assuming > it's somehow necessary for the stuff later in the series, but I think > we need a brief rationale here to explain why this isn't just adding > extra reg copies for the sake of it. Note that while this introduces extra opcodes, in many cases it does not change the number of machine instructions that are generated. Recall that accessing cpu_fpr[N] implies a load from env. This change makes the load explicit. The change does currently prevent caching cpu_fpr[N] in a host register. That can and will be fixed by optimizing on memory operations instead. (There is a patch that has been outstanding for 13 months to do this. I intend to finally get around to merging it during the 4.0 cycle.) r~
[Qemu-devel] [PATCH 8/9] tests: acpi: squash sanitize_fadt_ptrs() into test_acpi_fadt_table()
some parts of sanitize_fadt_ptrs() do redundant job
- locating FADT
- checking original checksum
There is no need to do it as test_acpi_fadt_table() already does that,
so drop duplicate code and move remaining fixup code into
test_acpi_fadt_table().
Signed-off-by: Igor Mammedov
---
tests/bios-tables-test.c | 40 ++--
1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 9aa7c86..40b224f 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -126,6 +126,7 @@ static void test_acpi_fadt_table(test_data *data)
/* FADT table is 1st */
AcpiSdtTable table = g_array_index(data->tables, typeof(table), 0);
uint8_t *fadt_aml = table.aml;
+uint32_t fadt_len = table.aml_len;
ACPI_ASSERT_CMP(table.header->signature, "FACP");
@@ -137,36 +138,17 @@ static void test_acpi_fadt_table(test_data *data)
acpi_fetch_table(, _len,
fadt_aml + 40 /* DSDT */, "DSDT", true);
g_array_append_val(data->tables, table);
-}
-
-static void sanitize_fadt_ptrs(test_data *data)
-{
-/* fixup pointers in FADT */
-int i;
-
-for (i = 0; i < data->tables->len; i++) {
-AcpiSdtTable *sdt = _array_index(data->tables, AcpiSdtTable, i);
-
-if (memcmp(>header->signature, "FACP", 4)) {
-continue;
-}
-/* check original FADT checksum before sanitizing table */
-g_assert(!acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len));
-
-/* sdt->aml field offset := spec offset - header size */
-memset(sdt->aml + 36, 0, 4); /* sanitize FIRMWARE_CTRL ptr */
-memset(sdt->aml + 40, 0, 4); /* sanitize DSDT ptr */
-if (sdt->header->revision >= 3) {
-memset(sdt->aml + 132, 0, 8); /* sanitize X_FIRMWARE_CTRL ptr */
-memset(sdt->aml + 140, 0, 8); /* sanitize X_DSDT ptr */
-}
-
-/* update checksum */
-sdt->header->checksum = 0;
-sdt->header->checksum -= acpi_calc_checksum(sdt->aml, sdt->aml_len);
-break;
+memset(fadt_aml + 36, 0, 4); /* sanitize FIRMWARE_CTRL ptr */
+memset(fadt_aml + 40, 0, 4); /* sanitize DSDT ptr */
+if (fadt_aml[8 /* FADT Major Version */] >= 3) {
+memset(fadt_aml + 132, 0, 8); /* sanitize X_FIRMWARE_CTRL ptr */
+memset(fadt_aml + 140, 0, 8); /* sanitize X_DSDT ptr */
}
+
+/* update checksum */
+fadt_aml[9 /* Checksum */] = 0;
+fadt_aml[9 /* Checksum */] -= acpi_calc_checksum(fadt_aml, fadt_len);
}
static void dump_aml_files(test_data *data, bool rebuild)
@@ -537,8 +519,6 @@ static void test_acpi_one(const char *params, test_data
*data)
test_acpi_rsdt_table(data);
test_acpi_fadt_table(data);
-sanitize_fadt_ptrs(data);
-
if (iasl) {
if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
dump_aml_files(data, true);
--
2.7.4
[Qemu-devel] [PATCH 4/9] tests: acpi: simplify rsdt handling
RSDT referenced tables always have length at offset 4 and checksum at
offset 9, that's enough for reusing fetch_table() and replacing custom
RSDT fetching code with it.
While at it
* merge fetch_rsdt_referenced_tables() into test_acpi_rsdt_table()
* drop test_data::rsdt_table/rsdt_tables_addr/rsdt_tables_nr since
we need this data only for duration of test_acpi_rsdt_table() to
fetch other tables and use locals instead.
Signed-off-by: Igor Mammedov
---
tests/bios-tables-test.c | 128 +++
1 file changed, 51 insertions(+), 77 deletions(-)
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 5faf75f..bea33a6 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -28,12 +28,9 @@ typedef struct {
const char *variant;
uint32_t rsdp_addr;
uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
-AcpiRsdtDescriptorRev1 rsdt_table;
uint32_t dsdt_addr;
uint32_t facs_addr;
AcpiFacsDescriptorRev1 facs_table;
-uint32_t *rsdt_tables_addr;
-int rsdt_tables_nr;
GArray *tables;
uint32_t smbios_ep_addr;
struct smbios_21_entry_point smbios_ep_table;
@@ -49,33 +46,48 @@ static const char *iasl = stringify(CONFIG_IASL);
static const char *iasl;
#endif
+static void cleanup_table_descriptor(AcpiSdtTable *table)
+{
+g_free(table->aml);
+if (table->aml_file &&
+!table->tmp_files_retain &&
+g_strstr_len(table->aml_file, -1, "aml-")) {
+unlink(table->aml_file);
+}
+g_free(table->aml_file);
+g_free(table->asl);
+if (table->asl_file &&
+!table->tmp_files_retain) {
+unlink(table->asl_file);
+}
+g_free(table->asl_file);
+}
+
static void free_test_data(test_data *data)
{
-AcpiSdtTable *temp;
int i;
-g_free(data->rsdt_tables_addr);
-
for (i = 0; i < data->tables->len; ++i) {
-temp = _array_index(data->tables, AcpiSdtTable, i);
-g_free(temp->aml);
-if (temp->aml_file &&
-!temp->tmp_files_retain &&
-g_strstr_len(temp->aml_file, -1, "aml-")) {
-unlink(temp->aml_file);
-}
-g_free(temp->aml_file);
-g_free(temp->asl);
-if (temp->asl_file &&
-!temp->tmp_files_retain) {
-unlink(temp->asl_file);
-}
-g_free(temp->asl_file);
+cleanup_table_descriptor(_array_index(data->tables, AcpiSdtTable,
i));
}
g_array_free(data->tables, true);
}
+/** fetch_table
+ * load ACPI table at @addr into table descriptor @sdt_table
+ * and check that header checksum matches actual one.
+ */
+static void fetch_table(AcpiSdtTable *sdt_table, uint32_t addr)
+{
+memread(addr + 4, _table->aml_len, 4); /* Length of ACPI table */
+sdt_table->aml_len = le32_to_cpu(sdt_table->aml_len);
+sdt_table->aml = g_malloc0(sdt_table->aml_len);
+memread(addr, sdt_table->aml, sdt_table->aml_len); /* get whole table */
+
+g_assert(!acpi_calc_checksum(sdt_table->aml, sdt_table->aml_len));
+}
+
static void test_acpi_rsdp_address(test_data *data)
{
uint32_t off = acpi_find_rsdp_address();
@@ -107,36 +119,31 @@ static void test_acpi_rsdp_table(test_data *data)
static void test_acpi_rsdt_table(test_data *data)
{
-AcpiRsdtDescriptorRev1 *rsdt_table = >rsdt_table;
uint32_t addr = acpi_get_rsdt_address(data->rsdp_table);
-uint32_t *tables;
-int tables_nr;
-uint8_t checksum;
-uint32_t rsdt_table_length;
+const int entry_size = 4 /* 32-bit Entry size */;
+const int tables_off = 36 /* 1st Entry */;
+AcpiSdtTable rsdt = {};
+int i, table_len, table_nr;
+uint32_t *entry;
/* read the header */
-ACPI_READ_TABLE_HEADER(rsdt_table, addr);
-ACPI_ASSERT_CMP(rsdt_table->signature, "RSDT");
-
-rsdt_table_length = le32_to_cpu(rsdt_table->length);
-
-/* compute the table entries in rsdt */
-tables_nr = (rsdt_table_length - sizeof(AcpiRsdtDescriptorRev1)) /
-sizeof(uint32_t);
-g_assert(tables_nr > 0);
+fetch_table(, addr);
+ACPI_ASSERT_CMP(rsdt.header->signature, "RSDT");
-/* get the addresses of the tables pointed by rsdt */
-tables = g_new0(uint32_t, tables_nr);
-ACPI_READ_ARRAY_PTR(tables, tables_nr, addr);
+/* Load all tables and add to test list directly RSDT referenced tables */
+table_len = le32_to_cpu(rsdt.header->length);
+table_nr = (table_len - tables_off) / entry_size;
+for (i = 0; i < table_nr; i++) {
+AcpiSdtTable ssdt_table = {};
-checksum = acpi_calc_checksum((uint8_t *)rsdt_table, rsdt_table_length) +
- acpi_calc_checksum((uint8_t *)tables,
- tables_nr * sizeof(uint32_t));
-g_assert(!checksum);
+entry = (uint32_t *)(rsdt.aml + tables_off + i * entry_size);
+addr = le32_to_cpu(*entry);
+fetch_table(_table, addr);
- /* SSDT tables after FADT */
-
[Qemu-devel] [PATCH 7/9] tests: smbios: fetch whole table in one step instead of reading it step by step
replace a bunch of ACPI_READ_ARRAY/ACPI_READ_FIELD macro, that read
SMBIOS table field by field with one memread() to fetch whole table
at once and drop no longer used ACPI_READ_ARRAY/ACPI_READ_FIELD macro.
Signed-off-by: Igor Mammedov
---
tests/acpi-utils.h | 17 -
tests/bios-tables-test.c | 15 +--
2 files changed, 1 insertion(+), 31 deletions(-)
diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index 1b584d3..bc0cd19 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -30,23 +30,6 @@ typedef struct {
bool tmp_files_retain; /* do not delete the temp asl/aml */
} AcpiSdtTable;
-#define ACPI_READ_FIELD(field, addr)\
-do {\
-memread(addr, , sizeof(field)); \
-addr += sizeof(field); \
-} while (0)
-
-#define ACPI_READ_ARRAY_PTR(arr, length, addr) \
-do {\
-int idx;\
-for (idx = 0; idx < length; ++idx) {\
-ACPI_READ_FIELD(arr[idx], addr);\
-} \
-} while (0)
-
-#define ACPI_READ_ARRAY(arr, addr) \
-ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr)
-
#define ACPI_ASSERT_CMP(actual, expected) do { \
char ACPI_ASSERT_CMP_str[5] = {}; \
memcpy(ACPI_ASSERT_CMP_str, , 4); \
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 8160fc0..9aa7c86 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -402,32 +402,19 @@ static bool smbios_ep_table_ok(test_data *data)
struct smbios_21_entry_point *ep_table = >smbios_ep_table;
uint32_t addr = data->smbios_ep_addr;
-ACPI_READ_ARRAY(ep_table->anchor_string, addr);
+memread(addr, ep_table, sizeof(*ep_table));
if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
return false;
}
-ACPI_READ_FIELD(ep_table->checksum, addr);
-ACPI_READ_FIELD(ep_table->length, addr);
-ACPI_READ_FIELD(ep_table->smbios_major_version, addr);
-ACPI_READ_FIELD(ep_table->smbios_minor_version, addr);
-ACPI_READ_FIELD(ep_table->max_structure_size, addr);
-ACPI_READ_FIELD(ep_table->entry_point_revision, addr);
-ACPI_READ_ARRAY(ep_table->formatted_area, addr);
-ACPI_READ_ARRAY(ep_table->intermediate_anchor_string, addr);
if (memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5)) {
return false;
}
-ACPI_READ_FIELD(ep_table->intermediate_checksum, addr);
-ACPI_READ_FIELD(ep_table->structure_table_length, addr);
if (ep_table->structure_table_length == 0) {
return false;
}
-ACPI_READ_FIELD(ep_table->structure_table_address, addr);
-ACPI_READ_FIELD(ep_table->number_of_structures, addr);
if (ep_table->number_of_structures == 0) {
return false;
}
-ACPI_READ_FIELD(ep_table->smbios_bcd_revision, addr);
if (acpi_calc_checksum((uint8_t *)ep_table, sizeof *ep_table) ||
acpi_calc_checksum((uint8_t *)ep_table + 0x10,
sizeof *ep_table - 0x10)) {
--
2.7.4
[Qemu-devel] [PATCH 1/9] tests: acpi: remove not used ACPI_READ_GENERIC_ADDRESS macro
Signed-off-by: Igor Mammedov
---
tests/acpi-utils.h | 8
1 file changed, 8 deletions(-)
diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index 4f4899d..c8844a2 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -70,14 +70,6 @@ typedef struct {
g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
} while (0)
-#define ACPI_READ_GENERIC_ADDRESS(field, addr) \
-do { \
-ACPI_READ_FIELD((field).space_id, addr); \
-ACPI_READ_FIELD((field).bit_width, addr);\
-ACPI_READ_FIELD((field).bit_offset, addr); \
-ACPI_READ_FIELD((field).access_width, addr); \
-ACPI_READ_FIELD((field).address, addr); \
-} while (0)
uint8_t acpi_calc_checksum(const uint8_t *data, int len);
--
2.7.4
[Qemu-devel] [PATCH 9/9] tests: acpi: use AcpiSdtTable::aml instead of AcpiSdtTable::header::signature
AcpiSdtTable::header::signature is the only remained field from
AcpiTableHeader structure used by tests. Instead of using packed
structure to access signature, just read it directly from table
blob and remove no longer used AcpiSdtTable::header / union and
keep only AcpiSdtTable::aml byte array.
Signed-off-by: Igor Mammedov
---
tests/acpi-utils.h | 6 +-
tests/bios-tables-test.c | 20 +---
2 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index bc0cd19..8d04f76 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -13,15 +13,11 @@
#ifndef TEST_ACPI_UTILS_H
#define TEST_ACPI_UTILS_H
-#include "hw/acpi/acpi-defs.h"
#include "libqtest.h"
/* DSDT and SSDTs format */
typedef struct {
-union {
-AcpiTableHeader *header;
-uint8_t *aml;/* aml bytecode from guest */
-};
+uint8_t *aml;/* aml bytecode from guest */
uint32_t aml_len;
gchar *aml_file;
gchar *asl;/* asl code generated from aml */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 40b224f..1a4e3b3 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -43,6 +43,11 @@ static const char *iasl = stringify(CONFIG_IASL);
static const char *iasl;
#endif
+static bool compare_signature(const AcpiSdtTable *sdt, const char *signature)
+{
+ return !memcmp(sdt->aml, signature, 4);
+}
+
static void cleanup_table_descriptor(AcpiSdtTable *table)
{
g_free(table->aml);
@@ -128,7 +133,7 @@ static void test_acpi_fadt_table(test_data *data)
uint8_t *fadt_aml = table.aml;
uint32_t fadt_len = table.aml_len;
-ACPI_ASSERT_CMP(table.header->signature, "FACP");
+g_assert(compare_signature(, "FACP"));
/* Since DSDT/FACS isn't in RSDT, add them to ASL test list manually */
acpi_fetch_table(, _len,
@@ -167,7 +172,7 @@ static void dump_aml_files(test_data *data, bool rebuild)
if (rebuild) {
aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
- (gchar *)>header->signature, ext);
+ sdt->aml, ext);
fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
} else {
@@ -185,11 +190,6 @@ static void dump_aml_files(test_data *data, bool rebuild)
}
}
-static bool compare_signature(AcpiSdtTable *sdt, const char *signature)
-{
- return !memcmp(>header->signature, signature, 4);
-}
-
static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
{
AcpiSdtTable *temp;
@@ -285,7 +285,7 @@ static GArray *load_expected_aml(test_data *data)
try_again:
aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine,
- (gchar *)>header->signature, ext);
+ sdt->aml, ext);
if (getenv("V")) {
fprintf(stderr, "\nLooking for expected file '%s'\n", aml_file);
}
@@ -345,14 +345,12 @@ static void test_acpi_asl(test_data *data)
fprintf(stderr,
"Warning! iasl couldn't parse the expected aml\n");
} else {
-uint32_t signature = cpu_to_le32(exp_sdt->header->signature);
sdt->tmp_files_retain = true;
exp_sdt->tmp_files_retain = true;
fprintf(stderr,
"acpi-test: Warning! %.4s mismatch. "
"Actual [asl:%s, aml:%s], Expected [asl:%s,
aml:%s].\n",
-(gchar *),
-sdt->asl_file, sdt->aml_file,
+exp_sdt->aml, sdt->asl_file, sdt->aml_file,
exp_sdt->asl_file, exp_sdt->aml_file);
if (getenv("V")) {
const char *diff_cmd = getenv("DIFF");
--
2.7.4
[Qemu-devel] [PATCH 6/9] tests: acpi: reuse fetch_table() in vmgenid-test
Move fetch_table() into acpi-utils.c renaming it to acpi_fetch_table()
and reuse it in vmgenid-test that reads RSDT and then tables it references,
to find and parse VMGNEID SSDT.
While at it wrap RSDT referenced tables enumeration into FOREACH macro
(similar to what we do with QLIST_FOREACH & co) to reuse it with bios and
vmgenid tests.
Signed-off-by: Igor Mammedov
---
tests/acpi-utils.h | 22 ++---
tests/acpi-utils.c | 33 +++--
tests/bios-tables-test.c | 48 +---
tests/vmgenid-test.c | 63 +++-
4 files changed, 62 insertions(+), 104 deletions(-)
diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index 2244e8e..1b584d3 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -22,7 +22,7 @@ typedef struct {
AcpiTableHeader *header;
uint8_t *aml;/* aml bytecode from guest */
};
-gsize aml_len;
+uint32_t aml_len;
gchar *aml_file;
gchar *asl;/* asl code generated from aml */
gsize asl_len;
@@ -47,19 +47,6 @@ typedef struct {
#define ACPI_READ_ARRAY(arr, addr) \
ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr)
-#define ACPI_READ_TABLE_HEADER(table, addr) \
-do { \
-ACPI_READ_FIELD((table)->signature, addr); \
-ACPI_READ_FIELD((table)->length, addr); \
-ACPI_READ_FIELD((table)->revision, addr);\
-ACPI_READ_FIELD((table)->checksum, addr);\
-ACPI_READ_ARRAY((table)->oem_id, addr); \
-ACPI_READ_ARRAY((table)->oem_table_id, addr);\
-ACPI_READ_FIELD((table)->oem_revision, addr);\
-ACPI_READ_ARRAY((table)->asl_compiler_id, addr); \
-ACPI_READ_FIELD((table)->asl_compiler_revision, addr); \
-} while (0)
-
#define ACPI_ASSERT_CMP(actual, expected) do { \
char ACPI_ASSERT_CMP_str[5] = {}; \
memcpy(ACPI_ASSERT_CMP_str, , 4); \
@@ -73,11 +60,16 @@ typedef struct {
} while (0)
+#define ACPI_FOREACH_RSDT_ENTRY(table, table_len, entry_ptr, entry_size) \
+for (entry_ptr = table + 36 /* 1st Entry */; \
+ entry_ptr < table + table_len; \
+ entry_ptr += entry_size)
uint8_t acpi_calc_checksum(const uint8_t *data, int len);
uint32_t acpi_find_rsdp_address(void);
-uint32_t acpi_get_rsdt_address(uint8_t *rsdp_table);
uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table);
void acpi_parse_rsdp_table(uint32_t addr, uint8_t *rsdp_table);
+void acpi_fetch_table(uint8_t **aml, uint32_t *aml_len, const uint8_t
*addr_ptr,
+ const char *sig, bool verify_checksum);
#endif /* TEST_ACPI_UTILS_H */
diff --git a/tests/acpi-utils.c b/tests/acpi-utils.c
index d07f748..de1492c 100644
--- a/tests/acpi-utils.c
+++ b/tests/acpi-utils.c
@@ -52,14 +52,6 @@ uint32_t acpi_find_rsdp_address(void)
return off;
}
-uint32_t acpi_get_rsdt_address(uint8_t *rsdp_table)
-{
-uint32_t rsdt_physical_address;
-
-memcpy(_physical_address, _table[16 /* RsdtAddress offset */],
4);
-return le32_to_cpu(rsdt_physical_address);
-}
-
uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table)
{
uint64_t xsdt_physical_address;
@@ -93,3 +85,28 @@ void acpi_parse_rsdp_table(uint32_t addr, uint8_t
*rsdp_table)
ACPI_ASSERT_CMP64(*((uint64_t *)(rsdp_table)), "RSD PTR ");
}
+
+/** acpi_fetch_table
+ * load ACPI table at @addr_ptr offset pointer into buffer and return it in
+ * @aml, its length in @aml_len and check that signature/checksum matches
+ * actual one.
+ */
+void acpi_fetch_table(uint8_t **aml, uint32_t *aml_len, const uint8_t
*addr_ptr,
+ const char *sig, bool verify_checksum)
+{
+uint32_t addr, len;
+
+memcpy(, addr_ptr , sizeof(addr));
+addr = le32_to_cpu(addr);
+memread(addr + 4, , 4); /* Length of ACPI table */
+*aml_len = le32_to_cpu(len);
+*aml = g_malloc0(*aml_len);
+memread(addr, *aml, *aml_len); /* get whole table */
+
+if (sig) {
+ACPI_ASSERT_CMP(**aml, sig);
+}
+if (verify_checksum) {
+g_assert(!acpi_calc_checksum(*aml, *aml_len));
+}
+}
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index df61fc2..8160fc0 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -71,30 +71,6 @@ static void free_test_data(test_data *data)
g_array_free(data->tables, true);
}
-/** fetch_table
- * load ACPI table at @addr_ptr offset pointer into table descriptor
- * @sdt_table and check that signature/checksum matches actual one.
- */
-static void fetch_table(AcpiSdtTable *sdt_table, uint8_t *addr_ptr,
-const char *sig, bool verify_checksum)
-{
-uint32_t addr;
-
-
[Qemu-devel] [PATCH 2/9] tests: acpi: use AcpiSdtTable::aml in consistent way
Currently in the 1st case we store table body fetched from QEMU in
AcpiSdtTable::aml minus it's header but in the 2nd case when we
load reference aml from disk, it holds whole blob including header.
More over in the 1st case, we read header in separate AcpiSdtTable::header
structure and then jump over hoops to fixup tables and combine both.
Treat AcpiSdtTable::aml as whole table blob approach in both cases
and when fetching tables from QEMU, first get table length and then
fetch whole table into AcpiSdtTable::aml instead if doing it field
by field.
As result
* AcpiSdtTable::aml is used in consistent manner
* FADT fixups use offsets from spec instead of being shifted by
header length
* calculating checksums and dumping blobs becomes simpler
Signed-off-by: Igor Mammedov
---
tests/acpi-utils.h | 6 +++--
tests/bios-tables-test.c | 59 +---
2 files changed, 25 insertions(+), 40 deletions(-)
diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index c8844a2..2244e8e 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -18,8 +18,10 @@
/* DSDT and SSDTs format */
typedef struct {
-AcpiTableHeader header;
-gchar *aml;/* aml bytecode from guest */
+union {
+AcpiTableHeader *header;
+uint8_t *aml;/* aml bytecode from guest */
+};
gsize aml_len;
gchar *aml_file;
gchar *asl;/* asl code generated from aml */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 8749b77..1666cf7 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -161,29 +161,24 @@ static void sanitize_fadt_ptrs(test_data *data)
for (i = 0; i < data->tables->len; i++) {
AcpiSdtTable *sdt = _array_index(data->tables, AcpiSdtTable, i);
-if (memcmp(>header.signature, "FACP", 4)) {
+if (memcmp(>header->signature, "FACP", 4)) {
continue;
}
/* check original FADT checksum before sanitizing table */
-g_assert(!(uint8_t)(
-acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
-acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len)
-));
+g_assert(!acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len));
/* sdt->aml field offset := spec offset - header size */
-memset(sdt->aml + 0, 0, 4); /* sanitize FIRMWARE_CTRL(36) ptr */
-memset(sdt->aml + 4, 0, 4); /* sanitize DSDT(40) ptr */
-if (sdt->header.revision >= 3) {
-memset(sdt->aml + 96, 0, 8); /* sanitize X_FIRMWARE_CTRL(132) ptr
*/
-memset(sdt->aml + 104, 0, 8); /* sanitize X_DSDT(140) ptr */
+memset(sdt->aml + 36, 0, 4); /* sanitize FIRMWARE_CTRL ptr */
+memset(sdt->aml + 40, 0, 4); /* sanitize DSDT ptr */
+if (sdt->header->revision >= 3) {
+memset(sdt->aml + 132, 0, 8); /* sanitize X_FIRMWARE_CTRL ptr */
+memset(sdt->aml + 140, 0, 8); /* sanitize X_DSDT ptr */
}
/* update checksum */
-sdt->header.checksum = 0;
-sdt->header.checksum -=
-acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
-acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len);
+sdt->header->checksum = 0;
+sdt->header->checksum -= acpi_calc_checksum(sdt->aml, sdt->aml_len);
break;
}
}
@@ -210,30 +205,21 @@ static void test_acpi_facs_table(test_data *data)
*/
static void fetch_table(AcpiSdtTable *sdt_table, uint32_t addr)
{
-uint8_t checksum;
-
-memset(sdt_table, 0, sizeof(*sdt_table));
-ACPI_READ_TABLE_HEADER(_table->header, addr);
-
-sdt_table->aml_len = le32_to_cpu(sdt_table->header.length)
- - sizeof(AcpiTableHeader);
+memread(addr + 4, _table->aml_len, 4); /* Length of ACPI table */
+sdt_table->aml_len = le32_to_cpu(sdt_table->aml_len);
sdt_table->aml = g_malloc0(sdt_table->aml_len);
-ACPI_READ_ARRAY_PTR(sdt_table->aml, sdt_table->aml_len, addr);
+memread(addr, sdt_table->aml, sdt_table->aml_len); /* get whole table */
-checksum = acpi_calc_checksum((uint8_t *)sdt_table,
- sizeof(AcpiTableHeader)) +
- acpi_calc_checksum((uint8_t *)sdt_table->aml,
- sdt_table->aml_len);
-g_assert(!checksum);
+g_assert(!acpi_calc_checksum(sdt_table->aml, sdt_table->aml_len));
}
static void test_acpi_dsdt_table(test_data *data)
{
-AcpiSdtTable dsdt_table;
+AcpiSdtTable dsdt_table = {};
uint32_t addr = le32_to_cpu(data->dsdt_addr);
fetch_table(_table, addr);
-ACPI_ASSERT_CMP(dsdt_table.header.signature, "DSDT");
+ACPI_ASSERT_CMP(dsdt_table.header->signature, "DSDT");
/* Since DSDT isn't in RSDT, add DSDT to ASL test tables list manually */
g_array_append_val(data->tables, dsdt_table);
@@ -246,7 +232,7 @@ static void
[Qemu-devel] [PATCH 5/9] tests: acpi: reuse fetch_table() for fetching FACS and DSDT
It allows to remove a bit more of code duplication and
reuse common utility to get ACPI tables from guest (modulo RSDP).
While at it, consolidate signature checking into fetch_table() instead
of open-codding it.
Considering FACS is special and doesn't have checksum, make checksum
validation optin, the same goes for signature verification.
PS:
By pure accident, patch also fixes FACS not being tested against
reference table since it wasn't added to data::tables list.
But we managed not to regress it since reference file was added
by commit
(d25979380 acpi unit-test: add test files)
back in 2013
Signed-off-by: Igor Mammedov
---
tests/bios-tables-test.c | 74 +---
1 file changed, 26 insertions(+), 48 deletions(-)
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index bea33a6..df61fc2 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -28,9 +28,6 @@ typedef struct {
const char *variant;
uint32_t rsdp_addr;
uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
-uint32_t dsdt_addr;
-uint32_t facs_addr;
-AcpiFacsDescriptorRev1 facs_table;
GArray *tables;
uint32_t smbios_ep_addr;
struct smbios_21_entry_point smbios_ep_table;
@@ -75,17 +72,27 @@ static void free_test_data(test_data *data)
}
/** fetch_table
- * load ACPI table at @addr into table descriptor @sdt_table
- * and check that header checksum matches actual one.
+ * load ACPI table at @addr_ptr offset pointer into table descriptor
+ * @sdt_table and check that signature/checksum matches actual one.
*/
-static void fetch_table(AcpiSdtTable *sdt_table, uint32_t addr)
+static void fetch_table(AcpiSdtTable *sdt_table, uint8_t *addr_ptr,
+const char *sig, bool verify_checksum)
{
+uint32_t addr;
+
+memcpy(, addr_ptr , sizeof(addr));
+addr = le32_to_cpu(addr);
memread(addr + 4, _table->aml_len, 4); /* Length of ACPI table */
sdt_table->aml_len = le32_to_cpu(sdt_table->aml_len);
sdt_table->aml = g_malloc0(sdt_table->aml_len);
memread(addr, sdt_table->aml, sdt_table->aml_len); /* get whole table */
-g_assert(!acpi_calc_checksum(sdt_table->aml, sdt_table->aml_len));
+if (sig) {
+ACPI_ASSERT_CMP(sdt_table->header->signature, sig);
+}
+if (verify_checksum) {
+g_assert(!acpi_calc_checksum(sdt_table->aml, sdt_table->aml_len));
+}
}
static void test_acpi_rsdp_address(test_data *data)
@@ -119,16 +126,13 @@ static void test_acpi_rsdp_table(test_data *data)
static void test_acpi_rsdt_table(test_data *data)
{
-uint32_t addr = acpi_get_rsdt_address(data->rsdp_table);
const int entry_size = 4 /* 32-bit Entry size */;
const int tables_off = 36 /* 1st Entry */;
AcpiSdtTable rsdt = {};
int i, table_len, table_nr;
-uint32_t *entry;
/* read the header */
-fetch_table(, addr);
-ACPI_ASSERT_CMP(rsdt.header->signature, "RSDT");
+fetch_table(, >rsdp_table[16 /* RsdtAddress */], "RSDT", true);
/* Load all tables and add to test list directly RSDT referenced tables */
table_len = le32_to_cpu(rsdt.header->length);
@@ -136,9 +140,8 @@ static void test_acpi_rsdt_table(test_data *data)
for (i = 0; i < table_nr; i++) {
AcpiSdtTable ssdt_table = {};
-entry = (uint32_t *)(rsdt.aml + tables_off + i * entry_size);
-addr = le32_to_cpu(*entry);
-fetch_table(_table, addr);
+fetch_table(_table, rsdt.aml + tables_off + i * entry_size,
+NULL, true);
/* Add table to ASL test tables list */
g_array_append_val(data->tables, ssdt_table);
@@ -149,12 +152,17 @@ static void test_acpi_rsdt_table(test_data *data)
static void test_acpi_fadt_table(test_data *data)
{
/* FADT table is 1st */
-AcpiSdtTable *fadt = _array_index(data->tables, typeof(*fadt), 0);
+AcpiSdtTable table = g_array_index(data->tables, typeof(table), 0);
+uint8_t *fadt_aml = table.aml;
+
+ACPI_ASSERT_CMP(table.header->signature, "FACP");
-ACPI_ASSERT_CMP(fadt->header->signature, "FACP");
+/* Since DSDT/FACS isn't in RSDT, add them to ASL test list manually */
+fetch_table(, fadt_aml + 36 /* FIRMWARE_CTRL */, "FACS", false);
+g_array_append_val(data->tables, table);
-memcpy(>facs_addr, fadt->aml + 36 /* FIRMWARE_CTRL */, 4);
-memcpy(>dsdt_addr, fadt->aml + 40 /* DSDT */, 4);
+fetch_table(, fadt_aml + 40 /* DSDT */, "DSDT", true);
+g_array_append_val(data->tables, table);
}
static void sanitize_fadt_ptrs(test_data *data)
@@ -187,34 +195,6 @@ static void sanitize_fadt_ptrs(test_data *data)
}
}
-static void test_acpi_facs_table(test_data *data)
-{
-AcpiFacsDescriptorRev1 *facs_table = >facs_table;
-uint32_t addr = le32_to_cpu(data->facs_addr);
-
-ACPI_READ_FIELD(facs_table->signature, addr);
-ACPI_READ_FIELD(facs_table->length, addr);
-
[Qemu-devel] [PATCH 0/9] tests: apci: consolidate and cleanup ACPI test code
While working on adding tests for virt/arm board (uefi/XSDT/64-bit table pointers), I found it's rather difficult to deal with mixed ACPI testing code that we've collected so far. So instead of just adding a pile of XSDT hacks on top, here goes small refactoring series: * that removes dead code * replaces reading tables with a fetch per table everywhere instead of mix of field by field and whole table * consolidates the way tables are read (reduces code duplication) * test no longer depends on ACPI structures from QEMU (i.e. doesn't affected by mistakes there) * fixiex FACS not beint compared against reference tables Overall test is reduced on ~170LOC and hopefully it makes easier to add more stuff on top. PS: Series depends on '[PATCH v3 0/8] hw: acpi: RSDP fixes and refactoring' to avoid nontrivial conflicts PS2: arm/virt test patches fill follow up a separate series on top of this one for not to mix things up CC: "Michael S. Tsirkin" CC: Thomas Huth CC: Laurent Vivier CC: Samuel Ortiz Igor Mammedov (9): tests: acpi: remove not used ACPI_READ_GENERIC_ADDRESS macro tests: acpi: use AcpiSdtTable::aml in consistent way tests: acpi: make sure FADT is fetched only once tests: acpi: simplify rsdt handling tests: acpi: reuse fetch_table() for fetching FACS and DSDT tests: acpi: reuse fetch_table() in vmgenid-test tests: smbios: fetch whole table in one step instead of reading it step by step tests: acpi: squash sanitize_fadt_ptrs() into test_acpi_fadt_table() tests: acpi: use AcpiSdtTable::aml instead of AcpiSdtTable::header::signature tests/acpi-utils.h | 51 ++ tests/acpi-utils.c | 33 -- tests/bios-tables-test.c | 257 --- tests/vmgenid-test.c | 63 4 files changed, 116 insertions(+), 288 deletions(-) -- 2.7.4
