Re: [PATCH 5/9] exec/address-spaces: Wrap address space singletons into functions
On 20/9/22 01:17, Bernhard Beschow wrote: In the next steps, these singletons will be resolved by turning them into attributes of the system bus. The system bus is already accessible via the global current_machine variable which will be made use of later in the wrapper functions. All changes have been performed with search-and-replace: * s/_space_memory/get_address_space_memory()/ * s/_space_io/get_address_space_io()/ The only exceptions were exec/address-spaces.h and softmmu/physmem.c which have been manually changed. Signed-off-by: Bernhard Beschow --- accel/hvf/hvf-accel-ops.c| 2 +- accel/kvm/kvm-all.c | 12 ++-- hw/alpha/dp264.c | 4 ++-- hw/alpha/typhoon.c | 4 ++-- hw/arm/smmu-common.c | 4 ++-- hw/arm/smmuv3.c | 14 +++--- hw/arm/virt.c| 2 +- hw/char/goldfish_tty.c | 4 ++-- hw/core/loader.c | 2 +- hw/dma/pl330.c | 2 +- hw/dma/rc4030.c | 2 +- hw/dma/xlnx-zynq-devcfg.c| 4 ++-- hw/dma/xlnx_dpdma.c | 8 hw/hppa/machine.c| 4 ++-- hw/hyperv/hyperv.c | 2 +- hw/hyperv/vmbus.c| 2 +- hw/i386/amd_iommu.c | 18 +- hw/i386/fw_cfg.c | 2 +- hw/i386/intel_iommu.c| 24 hw/i386/microvm.c| 4 ++-- hw/i386/pc.c | 2 +- hw/i386/xen/xen-hvm.c| 4 ++-- hw/ide/ahci.c| 2 +- hw/ide/macio.c | 10 +- hw/intc/apic.c | 2 +- hw/intc/openpic_kvm.c| 2 +- hw/intc/pnv_xive.c | 6 +++--- hw/intc/pnv_xive2.c | 6 +++--- hw/intc/riscv_aplic.c| 2 +- hw/intc/spapr_xive.c | 2 +- hw/intc/xive.c | 4 ++-- hw/intc/xive2.c | 4 ++-- hw/mips/jazz.c | 4 ++-- hw/misc/lasi.c | 2 +- hw/misc/macio/mac_dbdma.c| 8 hw/net/ftgmac100.c | 16 hw/net/i82596.c | 24 hw/net/imx_fec.c | 22 +++--- hw/net/lasi_i82596.c | 2 +- hw/net/npcm7xx_emc.c | 14 +++--- hw/openrisc/boot.c | 2 +- hw/pci-host/dino.c | 6 +++--- hw/pci-host/pnv_phb3.c | 6 +++--- hw/pci-host/pnv_phb3_msi.c | 6 +++--- hw/pci-host/pnv_phb4.c | 10 +- hw/pci/pci.c | 2 +- hw/ppc/pnv_psi.c | 2 +- hw/ppc/spapr.c | 4 ++-- hw/ppc/spapr_events.c| 2 +- hw/ppc/spapr_hcall.c | 4 ++-- hw/ppc/spapr_iommu.c | 4 ++-- hw/ppc/spapr_ovec.c | 8 hw/ppc/spapr_rtas.c | 2 +- hw/remote/iommu.c| 2 +- hw/remote/message.c | 4 ++-- hw/remote/proxy-memory-listener.c| 2 +- hw/riscv/boot.c | 6 +++--- hw/riscv/sifive_e.c | 2 +- hw/riscv/sifive_u.c | 2 +- hw/riscv/virt.c | 2 +- hw/s390x/css.c | 16 hw/s390x/ipl.h | 2 +- hw/s390x/s390-pci-bus.c | 4 ++-- hw/s390x/s390-pci-inst.c | 10 +- hw/s390x/s390-skeys.c| 2 +- hw/s390x/virtio-ccw.c| 10 +- hw/sd/sdhci.c| 2 +- hw/sh4/r2d.c | 4 ++-- hw/sparc/sun4m.c | 2 +- hw/sparc/sun4m_iommu.c | 4 ++-- hw/sparc64/sun4u_iommu.c | 4 ++-- hw/timer/hpet.c | 2 +- hw/usb/hcd-ehci-pci.c| 2 +- hw/usb/hcd-ehci-sysbus.c | 2 +- hw/usb/hcd-ohci.c| 2 +- hw/usb/hcd-xhci-sysbus.c | 2 +- hw/vfio/ap.c | 2 +- hw/vfio/ccw.c| 2 +- hw/vfio/common.c | 8 hw/vfio/platform.c | 2 +- hw/virtio/vhost-vdpa.c | 2 +- hw/virtio/vhost.c| 2 +- hw/virtio/virtio-bus.c | 4 ++-- hw/virtio/virtio-iommu.c | 6 +++--- hw/virtio/virtio-pci.c | 2 +- hw/xen/xen_pt.c | 4 ++-- include/exec/address-spaces.h| 4 ++-- include/hw/elf_ops.h | 4 ++-- include/hw/ppc/spapr.h
Re: [PATCH 9/9] exec/address-spaces: Inline legacy functions
On 20/9/22 07:15, Philippe Mathieu-Daudé wrote: On 20/9/22 01:17, Bernhard Beschow wrote: The functions just access a global pointer and perform some pointer arithmetic on top. Allow the compiler to see through this by inlining. I thought about this while reviewing the previous patch, ... Signed-off-by: Bernhard Beschow --- include/exec/address-spaces.h | 30 ++ softmmu/physmem.c | 28 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h index b31bd8dcf0..182af27cad 100644 --- a/include/exec/address-spaces.h +++ b/include/exec/address-spaces.h @@ -23,29 +23,51 @@ #ifndef CONFIG_USER_ONLY +#include "hw/boards.h" ... but I'm not a fan of including this header here. It is restricted to system emulation, but still... Let see what the others think. /** * Get the root memory region. This is a legacy function, provided for * compatibility. Prefer using SysBusState::system_memory directly. */ -MemoryRegion *get_system_memory(void); +inline MemoryRegion *get_system_memory(void) +{ + assert(current_machine); + + return _machine->main_system_bus.system_memory; +} Maybe we can simply declare them with __attribute__ ((const)) in the previous patch? See https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
Re: [PATCH v2] hw/virtio/vhost-shadow-virtqueue: Silence GCC error "maybe-uninitialized"
Am 10. September 2022 15:11:17 UTC schrieb Bernhard Beschow : >GCC issues a false positive warning, resulting in build failure with -Werror: > > In file included from /usr/include/glib-2.0/glib.h:114, > from src/include/glib-compat.h:32, > from src/include/qemu/osdep.h:144, > from ../src/hw/virtio/vhost-shadow-virtqueue.c:10: > In function ‘g_autoptr_cleanup_generic_gfree’, > inlined from ‘vhost_handle_guest_kick’ at > ../src/hw/virtio/vhost-shadow-virtqueue.c:292:42: > /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: ‘elem’ may be > used uninitialized [-Werror=maybe-uninitialized] > 28 | g_free (*pp); >| ^~~~ > ../src/hw/virtio/vhost-shadow-virtqueue.c: In function > ‘vhost_handle_guest_kick’: > ../src/hw/virtio/vhost-shadow-virtqueue.c:292:42: note: ‘elem’ was declared > here >292 | g_autofree VirtQueueElement *elem; >| ^~~~ > cc1: all warnings being treated as errors > >There is actually no problem since "elem" is initialized in both branches. >Silence the warning by initializig it with "NULL". > >$ gcc --version >gcc (GCC) 12.2.0 > >Fixes: 9c2ab2f1ec333be8614cc12272d4b91960704dbe ("vhost: stop transfer elem >ownership in vhost_handle_guest_kick") >Signed-off-by: Bernhard Beschow >--- Ping > hw/virtio/vhost-shadow-virtqueue.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/hw/virtio/vhost-shadow-virtqueue.c >b/hw/virtio/vhost-shadow-virtqueue.c >index e8e5bbc368..596d4434d2 100644 >--- a/hw/virtio/vhost-shadow-virtqueue.c >+++ b/hw/virtio/vhost-shadow-virtqueue.c >@@ -289,7 +289,7 @@ static void vhost_handle_guest_kick(VhostShadowVirtqueue >*svq) > virtio_queue_set_notification(svq->vq, false); > > while (true) { >-g_autofree VirtQueueElement *elem; >+g_autofree VirtQueueElement *elem = NULL; > int r; > > if (svq->next_guest_avail_elem) {
Re: [PATCH 9/9] exec/address-spaces: Inline legacy functions
On 20/9/22 01:17, Bernhard Beschow wrote: The functions just access a global pointer and perform some pointer arithmetic on top. Allow the compiler to see through this by inlining. I thought about this while reviewing the previous patch, ... Signed-off-by: Bernhard Beschow --- include/exec/address-spaces.h | 30 ++ softmmu/physmem.c | 28 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h index b31bd8dcf0..182af27cad 100644 --- a/include/exec/address-spaces.h +++ b/include/exec/address-spaces.h @@ -23,29 +23,51 @@ #ifndef CONFIG_USER_ONLY +#include "hw/boards.h" ... but I'm not a fan of including this header here. It is restricted to system emulation, but still... Let see what the others think. /** * Get the root memory region. This is a legacy function, provided for * compatibility. Prefer using SysBusState::system_memory directly. */ -MemoryRegion *get_system_memory(void); +inline MemoryRegion *get_system_memory(void) +{ +assert(current_machine); + +return _machine->main_system_bus.system_memory; +} /** * Get the root I/O port region. This is a legacy function, provided for * compatibility. Prefer using SysBusState::system_io directly. */ -MemoryRegion *get_system_io(void); +inline MemoryRegion *get_system_io(void) +{ +assert(current_machine); + +return _machine->main_system_bus.system_io; +} /** * Get the root memory address space. This is a legacy function, provided for * compatibility. Prefer using SysBusState::address_space_memory directly. */ -AddressSpace *get_address_space_memory(void); +inline AddressSpace *get_address_space_memory(void) +{ +assert(current_machine); + +return _machine->main_system_bus.address_space_memory; +} /** * Get the root I/O port address space. This is a legacy function, provided * for compatibility. Prefer using SysBusState::address_space_io directly. */ -AddressSpace *get_address_space_io(void); +inline AddressSpace *get_address_space_io(void) +{ +assert(current_machine); + +return _machine->main_system_bus.address_space_io; +} #endif diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 07e9a9171c..dce088f55c 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -2674,34 +2674,6 @@ static void memory_map_init(SysBusState *sysbus) address_space_init(>address_space_io, system_io, "I/O"); } -MemoryRegion *get_system_memory(void) -{ -assert(current_machine); - -return _machine->main_system_bus.system_memory; -} - -MemoryRegion *get_system_io(void) -{ -assert(current_machine); - -return _machine->main_system_bus.system_io; -} - -AddressSpace *get_address_space_memory(void) -{ -assert(current_machine); - -return _machine->main_system_bus.address_space_memory; -} - -AddressSpace *get_address_space_io(void) -{ -assert(current_machine); - -return _machine->main_system_bus.address_space_io; -} - static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr, hwaddr length) {
Re: [PATCH 8/9] softmmu/physmem: Let SysBusState absorb memory region and address space singletons
On 20/9/22 01:17, Bernhard Beschow wrote: These singletons are actually properties of the system bus but so far it hasn't been modelled that way. Fix this to make this relationship very obvious. The idea of the patch is to restrain futher proliferation of the use of get_system_memory() and get_system_io() which are "temprary interfaces" "further", "temporary" "until a proper bus interface is available". This should now be the case. Note that the new attributes are values rather than a pointers. This trades pointer dereferences for pointer arithmetic. The idea is to reduce cache misses - a rule of thumb says that every pointer dereference causes a cache miss while arithmetic is basically free. Signed-off-by: Bernhard Beschow --- include/exec/address-spaces.h | 19 --- include/hw/sysbus.h | 6 + softmmu/physmem.c | 46 ++- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h index d5c8cbd718..b31bd8dcf0 100644 --- a/include/exec/address-spaces.h +++ b/include/exec/address-spaces.h @@ -23,17 +23,28 @@ #ifndef CONFIG_USER_ONLY -/* Get the root memory region. This interface should only be used temporarily - * until a proper bus interface is available. +/** + * Get the root memory region. This is a legacy function, provided for + * compatibility. Prefer using SysBusState::system_memory directly. */ MemoryRegion *get_system_memory(void); diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h index 5bb3b88501..516e9091dc 100644 --- a/include/hw/sysbus.h +++ b/include/hw/sysbus.h @@ -17,6 +17,12 @@ struct SysBusState { /*< private >*/ BusState parent_obj; /*< public >*/ + +MemoryRegion system_memory; +MemoryRegion system_io; + +AddressSpace address_space_io; +AddressSpace address_space_memory; Alternatively (renaming doc accordingly): struct { MemoryRegion mr; AddressSpace as; } io, memory; }; #define TYPE_SYS_BUS_DEVICE "sys-bus-device" diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 0ac920d446..07e9a9171c 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -86,12 +86,6 @@ */ RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) }; -static MemoryRegion *system_memory; -static MemoryRegion *system_io; - -static AddressSpace address_space_io; -static AddressSpace address_space_memory; - static MemoryRegion io_mem_unassigned; typedef struct PhysPageEntry PhysPageEntry; @@ -146,7 +140,7 @@ typedef struct subpage_t { #define PHYS_SECTION_UNASSIGNED 0 static void io_mem_init(void); -static void memory_map_init(void); +static void memory_map_init(SysBusState *sysbus); static void tcg_log_global_after_sync(MemoryListener *listener); static void tcg_commit(MemoryListener *listener); @@ -2667,37 +2661,45 @@ static void tcg_commit(MemoryListener *listener) tlb_flush(cpuas->cpu); } -static void memory_map_init(void) +static void memory_map_init(SysBusState *sysbus) { No need to pass a singleton by argument. assert(current_machine); You can use get_system_memory() and get_system_io() in place :) LGTM otherwise, great! -system_memory = g_malloc(sizeof(*system_memory)); +MemoryRegion *system_memory = >system_memory; +MemoryRegion *system_io = >system_io; memory_region_init(system_memory, NULL, "system", UINT64_MAX); -address_space_init(_space_memory, system_memory, "memory"); +address_space_init(>address_space_memory, system_memory, "memory"); -system_io = g_malloc(sizeof(*system_io)); memory_region_init_io(system_io, NULL, _io_ops, NULL, "io", 65536); -address_space_init(_space_io, system_io, "I/O"); +address_space_init(>address_space_io, system_io, "I/O"); } MemoryRegion *get_system_memory(void) { -return system_memory; +assert(current_machine); + +return _machine->main_system_bus.system_memory; } MemoryRegion *get_system_io(void) { -return system_io; +assert(current_machine); + +return _machine->main_system_bus.system_io; } AddressSpace *get_address_space_memory(void) { -return _space_memory; +assert(current_machine); + +return _machine->main_system_bus.address_space_memory; } AddressSpace *get_address_space_io(void) { -return _space_io; +assert(current_machine); + +return _machine->main_system_bus.address_space_io; }
Re: [PATCH 6/9] target/loongarch/cpu: Remove unneeded include directive
On 20/9/22 01:17, Bernhard Beschow wrote: The cpu is used in both user and system emulation context while sysbus.h is system-only. Remove it since it's not needed anyway. Furthermore, it would cause a compile error in the next commit. Signed-off-by: Bernhard Beschow --- target/loongarch/cpu.h | 1 - 1 file changed, 1 deletion(-) diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h index dce999aaac..c9ed2cb3e7 100644 --- a/target/loongarch/cpu.h +++ b/target/loongarch/cpu.h @@ -13,7 +13,6 @@ #include "hw/registerfields.h" #include "qemu/timer.h" #include "exec/memory.h" -#include "hw/sysbus.h" #define IOCSRF_TEMP 0 #define IOCSRF_NODECNT 1 Renaming the subject as 'target: Remove unneeded "hw/sysbus.h" include directive' and fixing target/ppc/kvm.c: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 3/9] hw/core/sysbus: Resolve main_system_bus singleton
On 20/9/22 01:17, Bernhard Beschow wrote: In QEMU, a machine and the main_system_bus always go togehter. Usually the bus is part of the machine which suggsts to host it there. "together", "suggests" Since tere is already a current_machine singleton, all code that accesses the main_system_bus can be changed (behind the scenes) to go through current_machine. This resolves a singleton. Futhermore, by "Furthermore" reifying it in code, the every-machine-has-exactly-one-main-system-bus relationship becomes very obvious. Note that the main_system_bus attribute is a value rather than a pointer. This trades pointer dereferences for pointer arithmetic. The idea is to reduce cache misses - a rule of thumb says that every pointer dereference causes a cache miss while arithmetic is basically free. Signed-off-by: Bernhard Beschow --- hw/core/bus.c | 5 - hw/core/machine.c | 3 +++ hw/core/sysbus.c| 22 +- include/hw/boards.h | 1 + 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index 311ed17e18..7af940102d 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h Likely missing the BusState declaration: #include "hw/qdev-core.h" @@ -346,6 +346,7 @@ struct MachineState { */ MemoryRegion *ram; DeviceMemoryState *device_memory; +BusState main_system_bus; ram_addr_t ram_size; ram_addr_t maxram_size;
Re: [PATCH 2/9] exec/hwaddr.h: Add missing include
On 20/9/22 01:17, Bernhard Beschow wrote: The next commit would not compile w/o the include directive. Signed-off-by: Bernhard Beschow --- include/exec/hwaddr.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/exec/hwaddr.h b/include/exec/hwaddr.h index 8f16d179a8..616255317c 100644 --- a/include/exec/hwaddr.h +++ b/include/exec/hwaddr.h @@ -3,6 +3,7 @@ #ifndef HWADDR_H #define HWADDR_H +#include "qemu/osdep.h" NAck: This is an anti-pattern. "qemu/osdep.h" must not be included in .h, only in .c. Isn't including "hw/qdev-core.h" in "include/hw/boards.h" enough in the next patch?
Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
On 20/9/22 01:17, Bernhard Beschow wrote: SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to inherit from TYPE_MACHINE. This is an inconsistency which can cause undefined behavior such as memory corruption. Change SiFiveEState to inherit from MachineState since it is registered as a machine. Signed-off-by: Bernhard Beschow --- include/hw/riscv/sifive_e.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h index 83604da805..d738745925 100644 --- a/include/hw/riscv/sifive_e.h +++ b/include/hw/riscv/sifive_e.h @@ -22,6 +22,7 @@ #include "hw/riscv/riscv_hart.h" #include "hw/riscv/sifive_cpu.h" #include "hw/gpio/sifive_gpio.h" +#include "hw/boards.h" #define TYPE_RISCV_E_SOC "riscv.sifive.e.soc" #define RISCV_E_SOC(obj) \ @@ -41,7 +42,7 @@ typedef struct SiFiveESoCState { typedef struct SiFiveEState { /*< private >*/ -SysBusDevice parent_obj; +MachineState parent_obj; Ouch. Fixes: 0869490b1c ("riscv: sifive_e: Manually define the machine") Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] build: remove extra parentheses causing missing rebuilds
On 9/19/22 15:17, Paolo Bonzini wrote: Because of two stray parentheses at the end of the definition of ninja-cmd-goals, the test that is last in the .check-TESTSUITENAME.deps variable will not be rebuilt. Fix that. Signed-off-by: Paolo Bonzini --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] target/i386: fix INSERTQ implementation
On 9/18/22 09:56, Paolo Bonzini wrote: +else { +if (mod != 3) { +gen_lea_modrm(env, s, modrm); +op2_offset = offsetof(CPUX86State, xmm_t0); +gen_ldq_env_A0(s, offsetof(CPUX86State, xmm_t0.ZMM_D(0))); INSERTQ doesn't support a memory source. The two forms are INSERTQ xmm1, xmm2, imm8, imm8 INSERTQ xmm1, xmm2 r~
Re: [PATCH] target/i386: correctly mask SSE4a bit indices in register operands
On 9/18/22 09:18, Paolo Bonzini wrote: SSE4a instructions EXTRQ and INSERTQ have two bit index operands, that can be immediates or taken from an XMM register. In both cases, the fields are 6-bit wide and the top two bits in the byte are ignored. translate.c is doing that correctly for the immediate case, but not for the XMM case, so fix it. Signed-off-by: Paolo Bonzini --- target/i386/ops_sse.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson But these aren't SSE4a, they're AMD New Media instructions, which was a bit confusing. r~
Re: [PATCH] hw/virtio/vhost-user: support obtain vdpa device's mac address automatically
On Tue, Sep 13, 2022 at 5:13 PM Hao Chen wrote: > > When use dpdk-vdpa tests vdpa device. You need to specify the mac address to > start the virtual machine through libvirt or qemu, but now, the libvirt or > qemu can call dpdk vdpa vendor driver's ops .get_config through > vhost_net_get_config > to get the mac address of the vdpa hardware without manual configuration. > > Signed-off-by: Hao Chen Adding Cindy for comments. Thanks > --- > hw/block/vhost-user-blk.c | 1 - > hw/net/virtio-net.c | 3 ++- > hw/virtio/vhost-user.c| 19 --- > 3 files changed, 2 insertions(+), 21 deletions(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index 9117222456..5dca4eab09 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -337,7 +337,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error > **errp) > > vhost_dev_set_config_notifier(>dev, _ops); > > -s->vhost_user.supports_config = true; > ret = vhost_dev_init(>dev, >vhost_user, VHOST_BACKEND_TYPE_USER, 0, > errp); > if (ret < 0) { > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index dd0d056fde..274ea84644 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -149,7 +149,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, > uint8_t *config) > * Is this VDPA? No peer means not VDPA: there's no way to > * disconnect/reconnect a VDPA peer. > */ > -if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) { > +if ((nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) || > +(nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER)) { > ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t > *), > n->config_size); > if (ret != -1) { > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index bd24741be8..8b01078249 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -2013,8 +2013,6 @@ static int vhost_user_backend_init(struct vhost_dev > *dev, void *opaque, > } > > if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) { > -bool supports_f_config = vus->supports_config || > -(dev->config_ops && dev->config_ops->vhost_dev_config_notifier); > uint64_t protocol_features; > > dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES; > @@ -2033,23 +2031,6 @@ static int vhost_user_backend_init(struct vhost_dev > *dev, void *opaque, > */ > protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK; > > -if (supports_f_config) { > -if (!virtio_has_feature(protocol_features, > -VHOST_USER_PROTOCOL_F_CONFIG)) { > -error_setg(errp, "vhost-user device expecting " > - "VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user > backend does " > - "not support it."); > -return -EPROTO; > -} > -} else { > -if (virtio_has_feature(protocol_features, > - VHOST_USER_PROTOCOL_F_CONFIG)) { > -warn_reportf_err(*errp, "vhost-user backend supports " > - "VHOST_USER_PROTOCOL_F_CONFIG but QEMU does > not."); > -protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG); > -} > -} > - > /* final set of protocol features */ > dev->protocol_features = protocol_features; > err = vhost_user_set_protocol_features(dev, dev->protocol_features); > -- > 2.27.0 >
Re: [PATCH 5/5] target/loongarch: div if x/0 set dividend to 0
On 2022/9/19 19:45, gaosong wrote: 在 2022/9/17 下午6:12, Richard Henderson 写道: On 9/17/22 11:12, gaosong wrote: 在 2022/9/17 下午4:59, Qi Hu 写道: On 2022/9/17 15:59, Song Gao wrote: div.d, div.du, div,w, div.wu, the LoongArch host if x/0 the result is 0. The message has a typo: "div,w" => "div.w" Also I don't know why we need to do this, since the manual say: "When the divisor is 0, the result can be any value". I tested on LoongArch host, the result is always 0. But it is legal for a different loongarch host implementation to return some other value. Therefore the test itself is not correct. I think the manual maybe not correct, the hardware engineer said that they need to comfirm whether the result is always 0. Thanks. Song Gao Hi, The hardware designers suggested that 0 should not be used as the default value when "div 0" occurs. The behavior is not guaranteed in future processors. So I think there are some ways to solve this: - Remove this case("div 0") from risu test. - Keep this patch by yourself. If you want to do risu test, patch it. :-) regards, Qi r~
Re: [PATCH] virtio-net: set the max of queue size to 4096
On Tue, Sep 20, 2022 at 9:38 AM Jason Wang wrote: > > On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei wrote: > > > > From: liuhaiwei > > > > the limit of maximum of rx_queue_size and tx_queue to 1024 is so small as > > to affect our network performance when using the virtio-net and vhost , > > we cannot set the maximum size beyond 1k. > > why not enlarge the maximum size (such as 4096) when using the vhost > > backend? > > As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV. > We need to find way to overcome that limit first. Btw, this probably means the skb needs to be built by vhost-net itself, instead of tuntap. Thanks > > Thanks > > > > > Signed-off-by: liuhaiwei > > Signed-off-by: liuhaiwei > > --- > > hw/net/virtio-net.c| 47 +++--- > > hw/virtio/virtio.c | 8 +-- > > include/hw/virtio/virtio.h | 1 + > > 3 files changed, 41 insertions(+), 15 deletions(-) > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > index dd0d056fde..4b56484855 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -52,12 +52,11 @@ > > #define MAX_VLAN(1 << 12) /* Per 802.1Q definition */ > > > > /* previously fixed value */ > > -#define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 > > -#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 > > +#define VIRTIO_NET_VHOST_USER_DEFAULT_SIZE 2048 > > > > /* for now, only allow larger queue_pairs; with virtio-1, guest can > > downsize */ > > -#define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE > > -#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE > > +#define VIRTIO_NET_RX_QUEUE_MIN_SIZE 256 > > +#define VIRTIO_NET_TX_QUEUE_MIN_SIZE 256 > > > > #define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ > > > > @@ -594,6 +593,28 @@ static int peer_has_ufo(VirtIONet *n) > > return n->has_ufo; > > } > > > > +static void virtio_net_set_default_queue_size(VirtIONet *n) > > +{ > > +NetClientState *peer = n->nic_conf.peers.ncs[0]; > > + > > +/* Default value is 0 if not set */ > > +if (n->net_conf.rx_queue_size == 0) { > > +if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) { > > +n->net_conf.rx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE; > > +} else { > > +n->net_conf.rx_queue_size = VIRTIO_NET_VQ_MAX_SIZE; > > +} > > +} > > + > > +if (n->net_conf.tx_queue_size == 0) { > > +if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) { > > +n->net_conf.tx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE; > > +} else { > > +n->net_conf.tx_queue_size = VIRTIO_NET_VQ_MAX_SIZE; > > +} > > +} > > +} > > + > > static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, > > int version_1, int hash_report) > > { > > @@ -633,7 +654,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n) > > * size. > > */ > > if (!peer) { > > -return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; > > +return VIRTIO_NET_VQ_MAX_SIZE; > > } > > > > switch(peer->info->type) { > > @@ -641,7 +662,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n) > > case NET_CLIENT_DRIVER_VHOST_VDPA: > > return VIRTQUEUE_MAX_SIZE; > > default: > > -return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; > > +return VIRTIO_NET_VQ_MAX_SIZE; > > }; > > } > > > > @@ -3450,30 +3471,30 @@ static void virtio_net_device_realize(DeviceState > > *dev, Error **errp) > > > > virtio_net_set_config_size(n, n->host_features); > > virtio_init(vdev, VIRTIO_ID_NET, n->config_size); > > - > > +virtio_net_set_default_queue_size(n); > > /* > > * We set a lower limit on RX queue size to what it always was. > > * Guests that want a smaller ring can always resize it without > > * help from us (using virtio 1 and up). > > */ > > if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE || > > -n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE || > > +n->net_conf.rx_queue_size > VIRTIO_NET_VQ_MAX_SIZE || > > !is_power_of_2(n->net_conf.rx_queue_size)) { > > error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), " > > "must be a power of 2 between %d and %d.", > > n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE, > > - VIRTQUEUE_MAX_SIZE); > > + VIRTIO_NET_VQ_MAX_SIZE ); > > virtio_cleanup(vdev); > > return; > > } > > > > if (n->net_conf.tx_queue_size < VIRTIO_NET_TX_QUEUE_MIN_SIZE || > > -n->net_conf.tx_queue_size > VIRTQUEUE_MAX_SIZE || > > +n->net_conf.tx_queue_size > VIRTIO_NET_VQ_MAX_SIZE || > > !is_power_of_2(n->net_conf.tx_queue_size)) { > > error_setg(errp, "Invalid tx_queue_size (= %" PRIu16 "), " > >
Re: [PATCH] virtio-net: set the max of queue size to 4096
On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei wrote: > > From: liuhaiwei > > the limit of maximum of rx_queue_size and tx_queue to 1024 is so small as to > affect our network performance when using the virtio-net and vhost , > we cannot set the maximum size beyond 1k. > why not enlarge the maximum size (such as 4096) when using the vhost backend? As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV. We need to find way to overcome that limit first. Thanks > > Signed-off-by: liuhaiwei > Signed-off-by: liuhaiwei > --- > hw/net/virtio-net.c| 47 +++--- > hw/virtio/virtio.c | 8 +-- > include/hw/virtio/virtio.h | 1 + > 3 files changed, 41 insertions(+), 15 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index dd0d056fde..4b56484855 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -52,12 +52,11 @@ > #define MAX_VLAN(1 << 12) /* Per 802.1Q definition */ > > /* previously fixed value */ > -#define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 > -#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 > +#define VIRTIO_NET_VHOST_USER_DEFAULT_SIZE 2048 > > /* for now, only allow larger queue_pairs; with virtio-1, guest can downsize > */ > -#define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE > -#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE > +#define VIRTIO_NET_RX_QUEUE_MIN_SIZE 256 > +#define VIRTIO_NET_TX_QUEUE_MIN_SIZE 256 > > #define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ > > @@ -594,6 +593,28 @@ static int peer_has_ufo(VirtIONet *n) > return n->has_ufo; > } > > +static void virtio_net_set_default_queue_size(VirtIONet *n) > +{ > +NetClientState *peer = n->nic_conf.peers.ncs[0]; > + > +/* Default value is 0 if not set */ > +if (n->net_conf.rx_queue_size == 0) { > +if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) { > +n->net_conf.rx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE; > +} else { > +n->net_conf.rx_queue_size = VIRTIO_NET_VQ_MAX_SIZE; > +} > +} > + > +if (n->net_conf.tx_queue_size == 0) { > +if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) { > +n->net_conf.tx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE; > +} else { > +n->net_conf.tx_queue_size = VIRTIO_NET_VQ_MAX_SIZE; > +} > +} > +} > + > static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, > int version_1, int hash_report) > { > @@ -633,7 +654,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n) > * size. > */ > if (!peer) { > -return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; > +return VIRTIO_NET_VQ_MAX_SIZE; > } > > switch(peer->info->type) { > @@ -641,7 +662,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n) > case NET_CLIENT_DRIVER_VHOST_VDPA: > return VIRTQUEUE_MAX_SIZE; > default: > -return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; > +return VIRTIO_NET_VQ_MAX_SIZE; > }; > } > > @@ -3450,30 +3471,30 @@ static void virtio_net_device_realize(DeviceState > *dev, Error **errp) > > virtio_net_set_config_size(n, n->host_features); > virtio_init(vdev, VIRTIO_ID_NET, n->config_size); > - > +virtio_net_set_default_queue_size(n); > /* > * We set a lower limit on RX queue size to what it always was. > * Guests that want a smaller ring can always resize it without > * help from us (using virtio 1 and up). > */ > if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE || > -n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE || > +n->net_conf.rx_queue_size > VIRTIO_NET_VQ_MAX_SIZE || > !is_power_of_2(n->net_conf.rx_queue_size)) { > error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), " > "must be a power of 2 between %d and %d.", > n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE, > - VIRTQUEUE_MAX_SIZE); > + VIRTIO_NET_VQ_MAX_SIZE ); > virtio_cleanup(vdev); > return; > } > > if (n->net_conf.tx_queue_size < VIRTIO_NET_TX_QUEUE_MIN_SIZE || > -n->net_conf.tx_queue_size > VIRTQUEUE_MAX_SIZE || > +n->net_conf.tx_queue_size > VIRTIO_NET_VQ_MAX_SIZE || > !is_power_of_2(n->net_conf.tx_queue_size)) { > error_setg(errp, "Invalid tx_queue_size (= %" PRIu16 "), " > "must be a power of 2 between %d and %d", > n->net_conf.tx_queue_size, VIRTIO_NET_TX_QUEUE_MIN_SIZE, > - VIRTQUEUE_MAX_SIZE); > + VIRTIO_NET_VQ_MAX_SIZE); > virtio_cleanup(vdev); > return; > } > @@ -3751,9 +3772,9 @@ static Property virtio_net_properties[] = { > DEFINE_PROP_INT32("x-txburst", VirtIONet,
[PATCH] virtio-net: set the max of queue size to 4096
From: liuhaiwei the limit of maximum of rx_queue_size and tx_queue to 1024 is so small as to affect our network performance when using the virtio-net and vhost , we cannot set the maximum size beyond 1k. why not enlarge the maximum size (such as 4096) when using the vhost backend? Signed-off-by: liuhaiwei Signed-off-by: liuhaiwei --- hw/net/virtio-net.c| 47 +++--- hw/virtio/virtio.c | 8 +-- include/hw/virtio/virtio.h | 1 + 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index dd0d056fde..4b56484855 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -52,12 +52,11 @@ #define MAX_VLAN(1 << 12) /* Per 802.1Q definition */ /* previously fixed value */ -#define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 -#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 +#define VIRTIO_NET_VHOST_USER_DEFAULT_SIZE 2048 /* for now, only allow larger queue_pairs; with virtio-1, guest can downsize */ -#define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE -#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE +#define VIRTIO_NET_RX_QUEUE_MIN_SIZE 256 +#define VIRTIO_NET_TX_QUEUE_MIN_SIZE 256 #define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ @@ -594,6 +593,28 @@ static int peer_has_ufo(VirtIONet *n) return n->has_ufo; } +static void virtio_net_set_default_queue_size(VirtIONet *n) +{ +NetClientState *peer = n->nic_conf.peers.ncs[0]; + +/* Default value is 0 if not set */ +if (n->net_conf.rx_queue_size == 0) { +if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) { +n->net_conf.rx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE; +} else { +n->net_conf.rx_queue_size = VIRTIO_NET_VQ_MAX_SIZE; +} +} + +if (n->net_conf.tx_queue_size == 0) { +if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) { +n->net_conf.tx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE; +} else { +n->net_conf.tx_queue_size = VIRTIO_NET_VQ_MAX_SIZE; +} +} +} + static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, int version_1, int hash_report) { @@ -633,7 +654,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n) * size. */ if (!peer) { -return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; +return VIRTIO_NET_VQ_MAX_SIZE; } switch(peer->info->type) { @@ -641,7 +662,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n) case NET_CLIENT_DRIVER_VHOST_VDPA: return VIRTQUEUE_MAX_SIZE; default: -return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; +return VIRTIO_NET_VQ_MAX_SIZE; }; } @@ -3450,30 +3471,30 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) virtio_net_set_config_size(n, n->host_features); virtio_init(vdev, VIRTIO_ID_NET, n->config_size); - +virtio_net_set_default_queue_size(n); /* * We set a lower limit on RX queue size to what it always was. * Guests that want a smaller ring can always resize it without * help from us (using virtio 1 and up). */ if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE || -n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE || +n->net_conf.rx_queue_size > VIRTIO_NET_VQ_MAX_SIZE || !is_power_of_2(n->net_conf.rx_queue_size)) { error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), " "must be a power of 2 between %d and %d.", n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE, - VIRTQUEUE_MAX_SIZE); + VIRTIO_NET_VQ_MAX_SIZE ); virtio_cleanup(vdev); return; } if (n->net_conf.tx_queue_size < VIRTIO_NET_TX_QUEUE_MIN_SIZE || -n->net_conf.tx_queue_size > VIRTQUEUE_MAX_SIZE || +n->net_conf.tx_queue_size > VIRTIO_NET_VQ_MAX_SIZE || !is_power_of_2(n->net_conf.tx_queue_size)) { error_setg(errp, "Invalid tx_queue_size (= %" PRIu16 "), " "must be a power of 2 between %d and %d", n->net_conf.tx_queue_size, VIRTIO_NET_TX_QUEUE_MIN_SIZE, - VIRTQUEUE_MAX_SIZE); + VIRTIO_NET_VQ_MAX_SIZE); virtio_cleanup(vdev); return; } @@ -3751,9 +3772,9 @@ static Property virtio_net_properties[] = { DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST), DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx), DEFINE_PROP_UINT16("rx_queue_size", VirtIONet, net_conf.rx_queue_size, - VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE), + 0), DEFINE_PROP_UINT16("tx_queue_size", VirtIONet, net_conf.tx_queue_size, - VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE), + 0),
Re: [PATCH 1/2] include: import virtio_blk headers from linux with zoned device support
Stefan Hajnoczi 于2022年9月20日周二 03:59写道: > > On Sat, Sep 10, 2022 at 02:50:56PM +0800, Sam Li wrote: > > Add file from Dmitry's "virtio-blk:add support for zoned block devices" > > linux patch using scripts/update-linux-headers.sh. There is a link for > > more information: https://github.com/dmitry-fomichev/virtblk-zbd > > Hi Sam, > Linux headers are imported into QEMU using > scripts/update-linux-headers.sh. Did you import the header using this > script? > > If yes, please mention it in the commit description. If not, please do > so in the next revision. Yes, I'll change the commit description to "include: update virtio-blk header from Linux 5.19-rc2+". > > Thanks, > Stefan > > > > > Signed-off-by: Sam Li > > --- > > include/standard-headers/linux/virtio_blk.h | 109 > > 1 file changed, 109 insertions(+) > > > > diff --git a/include/standard-headers/linux/virtio_blk.h > > b/include/standard-headers/linux/virtio_blk.h > > index 2dcc90826a..490bd21c76 100644 > > --- a/include/standard-headers/linux/virtio_blk.h > > +++ b/include/standard-headers/linux/virtio_blk.h > > @@ -40,6 +40,7 @@ > > #define VIRTIO_BLK_F_MQ 12 /* support more than one vq */ > > #define VIRTIO_BLK_F_DISCARD 13 /* DISCARD is supported */ > > #define VIRTIO_BLK_F_WRITE_ZEROES14 /* WRITE ZEROES is supported > > */ > > +#define VIRTIO_BLK_F_ZONED 17 /* Zoned block device */ > > > > /* Legacy feature bits */ > > #ifndef VIRTIO_BLK_NO_LEGACY > > @@ -119,6 +120,20 @@ struct virtio_blk_config { > > uint8_t write_zeroes_may_unmap; > > > > uint8_t unused1[3]; > > + > > + /* Secure erase fields that are defined in the virtio spec */ > > + uint8_t sec_erase[12]; > > + > > + /* Zoned block device characteristics (if VIRTIO_BLK_F_ZONED) */ > > + struct virtio_blk_zoned_characteristics { > > + __virtio32 zone_sectors; > > + __virtio32 max_open_zones; > > + __virtio32 max_active_zones; > > + __virtio32 max_append_sectors; > > + __virtio32 write_granularity; > > + uint8_t model; > > + uint8_t unused2[3]; > > + } zoned; > > } QEMU_PACKED; > > > > /* > > @@ -153,6 +168,27 @@ struct virtio_blk_config { > > /* Write zeroes command */ > > #define VIRTIO_BLK_T_WRITE_ZEROES13 > > > > +/* Zone append command */ > > +#define VIRTIO_BLK_T_ZONE_APPEND15 > > + > > +/* Report zones command */ > > +#define VIRTIO_BLK_T_ZONE_REPORT16 > > + > > +/* Open zone command */ > > +#define VIRTIO_BLK_T_ZONE_OPEN 18 > > + > > +/* Close zone command */ > > +#define VIRTIO_BLK_T_ZONE_CLOSE 20 > > + > > +/* Finish zone command */ > > +#define VIRTIO_BLK_T_ZONE_FINISH22 > > + > > +/* Reset zone command */ > > +#define VIRTIO_BLK_T_ZONE_RESET 24 > > + > > +/* Reset All zones command */ > > +#define VIRTIO_BLK_T_ZONE_RESET_ALL 26 > > + > > #ifndef VIRTIO_BLK_NO_LEGACY > > /* Barrier before this op. */ > > #define VIRTIO_BLK_T_BARRIER 0x8000 > > @@ -172,6 +208,72 @@ struct virtio_blk_outhdr { > > __virtio64 sector; > > }; > > > > +/* > > + * Supported zoned device models. > > + */ > > + > > +/* Regular block device */ > > +#define VIRTIO_BLK_Z_NONE 0 > > +/* Host-managed zoned device */ > > +#define VIRTIO_BLK_Z_HM1 > > +/* Host-aware zoned device */ > > +#define VIRTIO_BLK_Z_HA2 > > + > > +/* > > + * Zone descriptor. A part of VIRTIO_BLK_T_ZONE_REPORT command reply. > > + */ > > +struct virtio_blk_zone_descriptor { > > + /* Zone capacity */ > > + __virtio64 z_cap; > > + /* The starting sector of the zone */ > > + __virtio64 z_start; > > + /* Zone write pointer position in sectors */ > > + __virtio64 z_wp; > > + /* Zone type */ > > + uint8_t z_type; > > + /* Zone state */ > > + uint8_t z_state; > > + uint8_t reserved[38]; > > +}; > > + > > +struct virtio_blk_zone_report { > > + __virtio64 nr_zones; > > + uint8_t reserved[56]; > > + struct virtio_blk_zone_descriptor zones[]; > > +}; > > + > > +/* > > + * Supported zone types. > > + */ > > + > > +/* Conventional zone */ > > +#define VIRTIO_BLK_ZT_CONV 1 > > +/* Sequential Write Required zone */ > > +#define VIRTIO_BLK_ZT_SWR 2 > > +/* Sequential Write Preferred zone */ > > +#define VIRTIO_BLK_ZT_SWP 3 > > + > > +/* > > + * Zone states that are available for zones of all types. > > + */ > > + > > +/* Not a write pointer (conventional zones only) */ > > +#define VIRTIO_BLK_ZS_NOT_WP 0 > > +/* Empty */ > > +#define VIRTIO_BLK_ZS_EMPTY1 > > +/* Implicitly Open */ > > +#define VIRTIO_BLK_ZS_IOPEN2 > > +/* Explicitly Open */ > > +#define VIRTIO_BLK_ZS_EOPEN3 > > +/* Closed */ > > +#define VIRTIO_BLK_ZS_CLOSED 4 > > +/* Read-Only */ > > +#define VIRTIO_BLK_ZS_RDONLY 13 > > +/* Full */ > > +#define VIRTIO_BLK_ZS_FULL 14 > > +/* Offline */ > > +#define
Re: [PATCH] target/riscv: Check the correct exception cause in vector GDB stub
On Sun, Sep 18, 2022 at 6:29 PM wrote: > > From: Frank Chang > > After RISCVException enum is introduced, riscv_csrrw_debug() returns > RISCV_EXCP_NONE to indicate there's no error. RISC-V vector GDB stub > should check the result against RISCV_EXCP_NONE instead of value 0. > Otherwise, 'E14' packet would be incorrectly reported for vector CSRs > when using "info reg vector" GDB command. > > Signed-off-by: Frank Chang > Reviewed-by: Jim Shu > Reviewed-by: Tommy Wu Thanks! Applied to riscv-to-apply.next Alistair > --- > target/riscv/gdbstub.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > index 9ed049c29e..118bd40f10 100644 > --- a/target/riscv/gdbstub.c > +++ b/target/riscv/gdbstub.c > @@ -211,7 +211,7 @@ static int riscv_gdb_get_vector(CPURISCVState *env, > GByteArray *buf, int n) > target_ulong val = 0; > int result = riscv_csrrw_debug(env, csrno, , 0, 0); > > -if (result == 0) { > +if (result == RISCV_EXCP_NONE) { > return gdb_get_regl(buf, val); > } > > @@ -238,7 +238,7 @@ static int riscv_gdb_set_vector(CPURISCVState *env, > uint8_t *mem_buf, int n) > target_ulong val = ldtul_p(mem_buf); > int result = riscv_csrrw_debug(env, csrno, NULL, val, -1); > > -if (result == 0) { > +if (result == RISCV_EXCP_NONE) { > return sizeof(target_ulong); > } > > -- > 2.36.1 > >
Re: [PATCH 0/3] hw/riscv: opentitan: Fixup resetvec issues
On Wed, Sep 14, 2022 at 8:11 PM Alistair Francis wrote: > > The OpenTitan resetvec is dynamic on QEMU as we don't run the full boot > ROM flow. This series makes it more configurguable from the command line > and fixes the default. > > Alistair Francis (3): > target/riscv: Set the CPU resetvec directly > hw/riscv: opentitan: Fixup resetvec > hw/riscv: opentitan: Expose the resetvec as a SoC property Thanks! Applied to riscv-to-apply.next Alistair > > include/hw/riscv/opentitan.h | 2 ++ > target/riscv/cpu.h | 3 +-- > hw/riscv/opentitan.c | 8 +++- > target/riscv/cpu.c | 13 +++-- > target/riscv/machine.c | 6 +++--- > 5 files changed, 16 insertions(+), 16 deletions(-) > > -- > 2.37.2 >
Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
On Tue, Sep 20, 2022 at 9:18 AM Bernhard Beschow wrote: > > SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to > inherit from TYPE_MACHINE. This is an inconsistency which can cause > undefined behavior such as memory corruption. > > Change SiFiveEState to inherit from MachineState since it is registered > as a machine. > > Signed-off-by: Bernhard Beschow Reviewed-by: Alistair Francis Alistair > --- > include/hw/riscv/sifive_e.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h > index 83604da805..d738745925 100644 > --- a/include/hw/riscv/sifive_e.h > +++ b/include/hw/riscv/sifive_e.h > @@ -22,6 +22,7 @@ > #include "hw/riscv/riscv_hart.h" > #include "hw/riscv/sifive_cpu.h" > #include "hw/gpio/sifive_gpio.h" > +#include "hw/boards.h" > > #define TYPE_RISCV_E_SOC "riscv.sifive.e.soc" > #define RISCV_E_SOC(obj) \ > @@ -41,7 +42,7 @@ typedef struct SiFiveESoCState { > > typedef struct SiFiveEState { > /*< private >*/ > -SysBusDevice parent_obj; > +MachineState parent_obj; > > /*< public >*/ > SiFiveESoCState soc; > -- > 2.37.3 > >
Re: [PATCH] target/riscv: Check the correct exception cause in vector GDB stub
On Sun, Sep 18, 2022 at 6:29 PM wrote: > > From: Frank Chang > > After RISCVException enum is introduced, riscv_csrrw_debug() returns > RISCV_EXCP_NONE to indicate there's no error. RISC-V vector GDB stub > should check the result against RISCV_EXCP_NONE instead of value 0. > Otherwise, 'E14' packet would be incorrectly reported for vector CSRs > when using "info reg vector" GDB command. > > Signed-off-by: Frank Chang > Reviewed-by: Jim Shu > Reviewed-by: Tommy Wu Reviewed-by: Alistair Francis Alistair > --- > target/riscv/gdbstub.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > index 9ed049c29e..118bd40f10 100644 > --- a/target/riscv/gdbstub.c > +++ b/target/riscv/gdbstub.c > @@ -211,7 +211,7 @@ static int riscv_gdb_get_vector(CPURISCVState *env, > GByteArray *buf, int n) > target_ulong val = 0; > int result = riscv_csrrw_debug(env, csrno, , 0, 0); > > -if (result == 0) { > +if (result == RISCV_EXCP_NONE) { > return gdb_get_regl(buf, val); > } > > @@ -238,7 +238,7 @@ static int riscv_gdb_set_vector(CPURISCVState *env, > uint8_t *mem_buf, int n) > target_ulong val = ldtul_p(mem_buf); > int result = riscv_csrrw_debug(env, csrno, NULL, val, -1); > > -if (result == 0) { > +if (result == RISCV_EXCP_NONE) { > return sizeof(target_ulong); > } > > -- > 2.36.1 > >
Re: [PATCH] target/riscv/pmp: fix non-translated page size address checks w/ MPU
On Sat, Sep 10, 2022 at 1:24 AM wrote: > > From: Leon Schuermann > > This commit fixes PMP address access checks with non page-aligned PMP > regions on harts with MPU enabled. Without this change, the presence > of an MPU in the virtual CPU model would influence the PMP address > check behavior when an access size was unknown (`size == 0`), > regardless of whether virtual memory has actually been enabled by the > guest. > > The RISC-V Privileged Spec Version 20211203[1] states in 4.3.1 > Addressing and Memory Protection that "[...] [w]hen Sv32 virtual > memory mode is selected in the MODE field of the satp register, > supervisor virtual addresses are translated into supervisor physical > addresses via a two-level page table. The 20-bit VPN is translated > into a 22-bit physical page number (PPN), while the 12-bit page offset > is untranslated. The resulting supervisor-level physical addresses are > then checked using any physical memory protection structures (Sections > 3.7), before being directly converted to machine-level physical > addresses. [...]" and "[...] [w]hen the value of satp.MODE is Bare, > the 32-bit virtual address is translated (unmodified) into a 32-bit > physical address [...]". Other modes such as Sv39, Sv48 and Sv57 are > said to behave similar in this regard. > > From this specification it can be inferred that any access made when > virtual memory is disabled, which is the case when satp.MODE is set to > "Bare" (0), should behave identically with respect to PMP checks as if > no MPU were present in the system at all. The current implementation, > however, degrades any PMP address checks of unknown access size (which > seems to be the case for instruction fetches at least) to be of > page-granularity, just based on the fact that the hart has MPU support > enabled. This causes systems that rely on 4-byte aligned PMP regions > to incur access faults, which are not occurring with the MPU disabled, > independent of any runtime guest configuration. > > While there possibly are other unhandled edge cases in which > page-granularity access checks might not be appropriate, this commit > appears to be a strict improvement over the current implementation's > behavior. It has been tested using Tock OS, but not with other > systems (e.g., Linux) yet. > > [1]: > https://github.com/riscv/riscv-isa-manual/releases/download/Priv-v1.12/riscv-privileged-20211203.pdf > > Signed-off-by: Leon Schuermann Reviewed-by: Alistair Francis > --- > > This patch is a resubmission to include all maintainers of the > modified files and main QEMU mailing list, as determined through the > `get_maintainer.pl` script. > > Also, one particular example of an additional edge case not handled > through this patch might be a hart operating in M-mode. Given that > virtual memory through {Sv32,Sv39,Sv48,Sv57} is only supported for > S-mode and U-mode respectively, enabling virtual memory in the satp > CSR should not have any effect on the behavior of memory accesses > w.r.t. PMP checks for harts operating in M-mode. > > I'm going to defer adding this additional check, as I'd appreciate some > feedback as to whether my reasoning is correct here at all first. > > Thanks! > > -Leon > > --- > target/riscv/pmp.c | 14 +++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index ea2b67d947..48f64a4aef 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -300,6 +300,7 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong > addr, > int i = 0; > int ret = -1; > int pmp_size = 0; > +uint64_t satp_mode; > target_ulong s = 0; > target_ulong e = 0; > > @@ -310,10 +311,17 @@ bool pmp_hart_has_privs(CPURISCVState *env, > target_ulong addr, > } > > if (size == 0) { > -if (riscv_feature(env, RISCV_FEATURE_MMU)) { > +if (riscv_cpu_mxl(env) == MXL_RV32) { > +satp_mode = SATP32_MODE; > +} else { > +satp_mode = SATP64_MODE; > +} > + > +if (riscv_feature(env, RISCV_FEATURE_MMU) > +&& get_field(env->satp, satp_mode)) { > /* > - * If size is unknown (0), assume that all bytes > - * from addr to the end of the page will be accessed. > + * If size is unknown (0) and virtual memory is enabled, assume > that > + * all bytes from addr to the end of the page will be accessed. > */ > pmp_size = -(addr | TARGET_PAGE_MASK); I'm not sure if we need this at all. This function is only called from get_physical_address_pmp() which then calculates the maximum size using pmp_is_range_in_tlb(). I suspect that we could just use sizeof(target_ulong) as the fallback for every time size == 0. Then pmp_is_range_in_tlb() will set the tlb_size to the maximum possible size of the PMP region. As a plus, we would remove some macros as well, so what about (untested)? if
[PATCH 8/9] softmmu/physmem: Let SysBusState absorb memory region and address space singletons
These singletons are actually properties of the system bus but so far it hasn't been modelled that way. Fix this to make this relationship very obvious. The idea of the patch is to restrain futher proliferation of the use of get_system_memory() and get_system_io() which are "temprary interfaces" "until a proper bus interface is available". This should now be the case. Note that the new attributes are values rather than a pointers. This trades pointer dereferences for pointer arithmetic. The idea is to reduce cache misses - a rule of thumb says that every pointer dereference causes a cache miss while arithmetic is basically free. Signed-off-by: Bernhard Beschow --- include/exec/address-spaces.h | 19 --- include/hw/sysbus.h | 6 + softmmu/physmem.c | 46 ++- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h index d5c8cbd718..b31bd8dcf0 100644 --- a/include/exec/address-spaces.h +++ b/include/exec/address-spaces.h @@ -23,17 +23,28 @@ #ifndef CONFIG_USER_ONLY -/* Get the root memory region. This interface should only be used temporarily - * until a proper bus interface is available. +/** + * Get the root memory region. This is a legacy function, provided for + * compatibility. Prefer using SysBusState::system_memory directly. */ MemoryRegion *get_system_memory(void); -/* Get the root I/O port region. This interface should only be used - * temporarily until a proper bus interface is available. +/** + * Get the root I/O port region. This is a legacy function, provided for + * compatibility. Prefer using SysBusState::system_io directly. */ MemoryRegion *get_system_io(void); +/** + * Get the root memory address space. This is a legacy function, provided for + * compatibility. Prefer using SysBusState::address_space_memory directly. + */ AddressSpace *get_address_space_memory(void); + +/** + * Get the root I/O port address space. This is a legacy function, provided + * for compatibility. Prefer using SysBusState::address_space_io directly. + */ AddressSpace *get_address_space_io(void); #endif diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h index 5bb3b88501..516e9091dc 100644 --- a/include/hw/sysbus.h +++ b/include/hw/sysbus.h @@ -17,6 +17,12 @@ struct SysBusState { /*< private >*/ BusState parent_obj; /*< public >*/ + +MemoryRegion system_memory; +MemoryRegion system_io; + +AddressSpace address_space_io; +AddressSpace address_space_memory; }; #define TYPE_SYS_BUS_DEVICE "sys-bus-device" diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 0ac920d446..07e9a9171c 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -86,12 +86,6 @@ */ RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list.blocks) }; -static MemoryRegion *system_memory; -static MemoryRegion *system_io; - -static AddressSpace address_space_io; -static AddressSpace address_space_memory; - static MemoryRegion io_mem_unassigned; typedef struct PhysPageEntry PhysPageEntry; @@ -146,7 +140,7 @@ typedef struct subpage_t { #define PHYS_SECTION_UNASSIGNED 0 static void io_mem_init(void); -static void memory_map_init(void); +static void memory_map_init(SysBusState *sysbus); static void tcg_log_global_after_sync(MemoryListener *listener); static void tcg_commit(MemoryListener *listener); @@ -2667,37 +2661,45 @@ static void tcg_commit(MemoryListener *listener) tlb_flush(cpuas->cpu); } -static void memory_map_init(void) +static void memory_map_init(SysBusState *sysbus) { -system_memory = g_malloc(sizeof(*system_memory)); +MemoryRegion *system_memory = >system_memory; +MemoryRegion *system_io = >system_io; memory_region_init(system_memory, NULL, "system", UINT64_MAX); -address_space_init(_space_memory, system_memory, "memory"); +address_space_init(>address_space_memory, system_memory, "memory"); -system_io = g_malloc(sizeof(*system_io)); memory_region_init_io(system_io, NULL, _io_ops, NULL, "io", 65536); -address_space_init(_space_io, system_io, "I/O"); +address_space_init(>address_space_io, system_io, "I/O"); } MemoryRegion *get_system_memory(void) { -return system_memory; +assert(current_machine); + +return _machine->main_system_bus.system_memory; } MemoryRegion *get_system_io(void) { -return system_io; +assert(current_machine); + +return _machine->main_system_bus.system_io; } AddressSpace *get_address_space_memory(void) { -return _space_memory; +assert(current_machine); + +return _machine->main_system_bus.address_space_memory; } AddressSpace *get_address_space_io(void) { -return _space_io; +assert(current_machine); + +return _machine->main_system_bus.address_space_io; } static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr, @@ -3003,7 +3005,7 @@
[PATCH 5/9] exec/address-spaces: Wrap address space singletons into functions
In the next steps, these singletons will be resolved by turning them into attributes of the system bus. The system bus is already accessible via the global current_machine variable which will be made use of later in the wrapper functions. All changes have been performed with search-and-replace: * s/_space_memory/get_address_space_memory()/ * s/_space_io/get_address_space_io()/ The only exceptions were exec/address-spaces.h and softmmu/physmem.c which have been manually changed. Signed-off-by: Bernhard Beschow --- accel/hvf/hvf-accel-ops.c| 2 +- accel/kvm/kvm-all.c | 12 ++-- hw/alpha/dp264.c | 4 ++-- hw/alpha/typhoon.c | 4 ++-- hw/arm/smmu-common.c | 4 ++-- hw/arm/smmuv3.c | 14 +++--- hw/arm/virt.c| 2 +- hw/char/goldfish_tty.c | 4 ++-- hw/core/loader.c | 2 +- hw/dma/pl330.c | 2 +- hw/dma/rc4030.c | 2 +- hw/dma/xlnx-zynq-devcfg.c| 4 ++-- hw/dma/xlnx_dpdma.c | 8 hw/hppa/machine.c| 4 ++-- hw/hyperv/hyperv.c | 2 +- hw/hyperv/vmbus.c| 2 +- hw/i386/amd_iommu.c | 18 +- hw/i386/fw_cfg.c | 2 +- hw/i386/intel_iommu.c| 24 hw/i386/microvm.c| 4 ++-- hw/i386/pc.c | 2 +- hw/i386/xen/xen-hvm.c| 4 ++-- hw/ide/ahci.c| 2 +- hw/ide/macio.c | 10 +- hw/intc/apic.c | 2 +- hw/intc/openpic_kvm.c| 2 +- hw/intc/pnv_xive.c | 6 +++--- hw/intc/pnv_xive2.c | 6 +++--- hw/intc/riscv_aplic.c| 2 +- hw/intc/spapr_xive.c | 2 +- hw/intc/xive.c | 4 ++-- hw/intc/xive2.c | 4 ++-- hw/mips/jazz.c | 4 ++-- hw/misc/lasi.c | 2 +- hw/misc/macio/mac_dbdma.c| 8 hw/net/ftgmac100.c | 16 hw/net/i82596.c | 24 hw/net/imx_fec.c | 22 +++--- hw/net/lasi_i82596.c | 2 +- hw/net/npcm7xx_emc.c | 14 +++--- hw/openrisc/boot.c | 2 +- hw/pci-host/dino.c | 6 +++--- hw/pci-host/pnv_phb3.c | 6 +++--- hw/pci-host/pnv_phb3_msi.c | 6 +++--- hw/pci-host/pnv_phb4.c | 10 +- hw/pci/pci.c | 2 +- hw/ppc/pnv_psi.c | 2 +- hw/ppc/spapr.c | 4 ++-- hw/ppc/spapr_events.c| 2 +- hw/ppc/spapr_hcall.c | 4 ++-- hw/ppc/spapr_iommu.c | 4 ++-- hw/ppc/spapr_ovec.c | 8 hw/ppc/spapr_rtas.c | 2 +- hw/remote/iommu.c| 2 +- hw/remote/message.c | 4 ++-- hw/remote/proxy-memory-listener.c| 2 +- hw/riscv/boot.c | 6 +++--- hw/riscv/sifive_e.c | 2 +- hw/riscv/sifive_u.c | 2 +- hw/riscv/virt.c | 2 +- hw/s390x/css.c | 16 hw/s390x/ipl.h | 2 +- hw/s390x/s390-pci-bus.c | 4 ++-- hw/s390x/s390-pci-inst.c | 10 +- hw/s390x/s390-skeys.c| 2 +- hw/s390x/virtio-ccw.c| 10 +- hw/sd/sdhci.c| 2 +- hw/sh4/r2d.c | 4 ++-- hw/sparc/sun4m.c | 2 +- hw/sparc/sun4m_iommu.c | 4 ++-- hw/sparc64/sun4u_iommu.c | 4 ++-- hw/timer/hpet.c | 2 +- hw/usb/hcd-ehci-pci.c| 2 +- hw/usb/hcd-ehci-sysbus.c | 2 +- hw/usb/hcd-ohci.c| 2 +- hw/usb/hcd-xhci-sysbus.c | 2 +- hw/vfio/ap.c | 2 +- hw/vfio/ccw.c| 2 +- hw/vfio/common.c | 8 hw/vfio/platform.c | 2 +- hw/virtio/vhost-vdpa.c | 2 +- hw/virtio/vhost.c| 2 +- hw/virtio/virtio-bus.c | 4 ++-- hw/virtio/virtio-iommu.c | 6 +++--- hw/virtio/virtio-pci.c | 2 +- hw/xen/xen_pt.c | 4 ++-- include/exec/address-spaces.h| 4 ++-- include/hw/elf_ops.h | 4 ++-- include/hw/ppc/spapr.h | 5 +++-- include/hw/ppc/vof.h | 4 ++-- monitor/misc.c | 4 ++--
[PATCH 9/9] exec/address-spaces: Inline legacy functions
The functions just access a global pointer and perform some pointer arithmetic on top. Allow the compiler to see through this by inlining. Signed-off-by: Bernhard Beschow --- include/exec/address-spaces.h | 30 ++ softmmu/physmem.c | 28 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h index b31bd8dcf0..182af27cad 100644 --- a/include/exec/address-spaces.h +++ b/include/exec/address-spaces.h @@ -23,29 +23,51 @@ #ifndef CONFIG_USER_ONLY +#include "hw/boards.h" + /** * Get the root memory region. This is a legacy function, provided for * compatibility. Prefer using SysBusState::system_memory directly. */ -MemoryRegion *get_system_memory(void); +inline MemoryRegion *get_system_memory(void) +{ +assert(current_machine); + +return _machine->main_system_bus.system_memory; +} /** * Get the root I/O port region. This is a legacy function, provided for * compatibility. Prefer using SysBusState::system_io directly. */ -MemoryRegion *get_system_io(void); +inline MemoryRegion *get_system_io(void) +{ +assert(current_machine); + +return _machine->main_system_bus.system_io; +} /** * Get the root memory address space. This is a legacy function, provided for * compatibility. Prefer using SysBusState::address_space_memory directly. */ -AddressSpace *get_address_space_memory(void); +inline AddressSpace *get_address_space_memory(void) +{ +assert(current_machine); + +return _machine->main_system_bus.address_space_memory; +} /** * Get the root I/O port address space. This is a legacy function, provided * for compatibility. Prefer using SysBusState::address_space_io directly. */ -AddressSpace *get_address_space_io(void); +inline AddressSpace *get_address_space_io(void) +{ +assert(current_machine); + +return _machine->main_system_bus.address_space_io; +} #endif diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 07e9a9171c..dce088f55c 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -2674,34 +2674,6 @@ static void memory_map_init(SysBusState *sysbus) address_space_init(>address_space_io, system_io, "I/O"); } -MemoryRegion *get_system_memory(void) -{ -assert(current_machine); - -return _machine->main_system_bus.system_memory; -} - -MemoryRegion *get_system_io(void) -{ -assert(current_machine); - -return _machine->main_system_bus.system_io; -} - -AddressSpace *get_address_space_memory(void) -{ -assert(current_machine); - -return _machine->main_system_bus.address_space_memory; -} - -AddressSpace *get_address_space_io(void) -{ -assert(current_machine); - -return _machine->main_system_bus.address_space_io; -} - static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr, hwaddr length) { -- 2.37.3
[PATCH 6/9] target/loongarch/cpu: Remove unneeded include directive
The cpu is used in both user and system emulation context while sysbus.h is system-only. Remove it since it's not needed anyway. Furthermore, it would cause a compile error in the next commit. Signed-off-by: Bernhard Beschow --- target/loongarch/cpu.h | 1 - 1 file changed, 1 deletion(-) diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h index dce999aaac..c9ed2cb3e7 100644 --- a/target/loongarch/cpu.h +++ b/target/loongarch/cpu.h @@ -13,7 +13,6 @@ #include "hw/registerfields.h" #include "qemu/timer.h" #include "exec/memory.h" -#include "hw/sysbus.h" #define IOCSRF_TEMP 0 #define IOCSRF_NODECNT 1 -- 2.37.3
[PATCH 4/9] hw/ppc/spapr: Fix code style problems reported by checkpatch
Signed-off-by: Bernhard Beschow --- include/hw/ppc/spapr.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 530d739b1d..04a95669ab 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -848,7 +848,8 @@ static inline uint64_t ppc64_phys_to_real(uint64_t addr) static inline uint32_t rtas_ld(target_ulong phys, int n) { -return ldl_be_phys(_space_memory, ppc64_phys_to_real(phys + 4*n)); +return ldl_be_phys(_space_memory, + ppc64_phys_to_real(phys + 4 * n)); } static inline uint64_t rtas_ldq(target_ulong phys, int n) @@ -858,7 +859,7 @@ static inline uint64_t rtas_ldq(target_ulong phys, int n) static inline void rtas_st(target_ulong phys, int n, uint32_t val) { -stl_be_phys(_space_memory, ppc64_phys_to_real(phys + 4*n), val); +stl_be_phys(_space_memory, ppc64_phys_to_real(phys + 4 * n), val); } typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, SpaprMachineState *sm, -- 2.37.3
[PATCH 3/9] hw/core/sysbus: Resolve main_system_bus singleton
In QEMU, a machine and the main_system_bus always go togehter. Usually the bus is part of the machine which suggsts to host it there. Since tere is already a current_machine singleton, all code that accesses the main_system_bus can be changed (behind the scenes) to go through current_machine. This resolves a singleton. Futhermore, by reifying it in code, the every-machine-has-exactly-one-main-system-bus relationship becomes very obvious. Note that the main_system_bus attribute is a value rather than a pointer. This trades pointer dereferences for pointer arithmetic. The idea is to reduce cache misses - a rule of thumb says that every pointer dereference causes a cache miss while arithmetic is basically free. Signed-off-by: Bernhard Beschow --- hw/core/bus.c | 5 - hw/core/machine.c | 3 +++ hw/core/sysbus.c| 22 +- include/hw/boards.h | 1 + 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/hw/core/bus.c b/hw/core/bus.c index c7831b5293..e3e807946c 100644 --- a/hw/core/bus.c +++ b/hw/core/bus.c @@ -129,9 +129,12 @@ static void qbus_init_internal(BusState *bus, DeviceState *parent, bus->parent->num_child_bus++; object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus)); object_unref(OBJECT(bus)); + +/* The only bus without a parent is the main system bus */ +assert(sysbus_get_default()); } else { /* The only bus without a parent is the main system bus */ -assert(bus == sysbus_get_default()); +assert(!sysbus_get_default()); } } diff --git a/hw/core/machine.c b/hw/core/machine.c index aa520e74a8..ebd3e0ff08 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -1097,6 +1097,9 @@ static void machine_initfn(Object *obj) ms->smp.threads = 1; machine_copy_boot_config(ms, &(BootConfiguration){ 0 }); + +qbus_init(>main_system_bus, sizeof(ms->main_system_bus), + TYPE_SYSTEM_BUS, NULL, "main-system-bus"); } static void machine_finalize(Object *obj) diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index 05c1da3d31..16a9b4d7a0 100644 --- a/hw/core/sysbus.c +++ b/hw/core/sysbus.c @@ -20,6 +20,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/module.h" +#include "hw/boards.h" #include "hw/sysbus.h" #include "monitor/monitor.h" #include "exec/address-spaces.h" @@ -336,26 +337,13 @@ static const TypeInfo sysbus_device_type_info = { .class_init = sysbus_device_class_init, }; -static BusState *main_system_bus; - -static void main_system_bus_create(void) -{ -/* - * assign main_system_bus before qbus_init() - * in order to make "if (bus != sysbus_get_default())" work - */ -main_system_bus = g_malloc0(system_bus_info.instance_size); -qbus_init(main_system_bus, system_bus_info.instance_size, - TYPE_SYSTEM_BUS, NULL, "main-system-bus"); -OBJECT(main_system_bus)->free = g_free; -} - BusState *sysbus_get_default(void) { -if (!main_system_bus) { -main_system_bus_create(); +if (!current_machine) { +return NULL; } -return main_system_bus; + +return _machine->main_system_bus; } static void sysbus_register_types(void) diff --git a/include/hw/boards.h b/include/hw/boards.h index 311ed17e18..7af940102d 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -346,6 +346,7 @@ struct MachineState { */ MemoryRegion *ram; DeviceMemoryState *device_memory; +BusState main_system_bus; ram_addr_t ram_size; ram_addr_t maxram_size; -- 2.37.3
[PATCH 7/9] hw/sysbus: Introduce dedicated struct SysBusState for TYPE_SYSTEM_BUS
With this out of the way, in the next step, SysBusState gains attributes for its memory and address recouces. Signed-off-by: Bernhard Beschow --- hw/core/sysbus.c | 4 ++-- include/hw/boards.h | 3 ++- include/hw/misc/macio/macio.h | 2 +- include/hw/sysbus.h | 8 ++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index 16a9b4d7a0..1100f3ad6c 100644 --- a/hw/core/sysbus.c +++ b/hw/core/sysbus.c @@ -84,7 +84,7 @@ static void system_bus_class_init(ObjectClass *klass, void *data) static const TypeInfo system_bus_info = { .name = TYPE_SYSTEM_BUS, .parent = TYPE_BUS, -.instance_size = sizeof(BusState), +.instance_size = sizeof(SysBusState), .class_init = system_bus_class_init, }; @@ -343,7 +343,7 @@ BusState *sysbus_get_default(void) return NULL; } -return _machine->main_system_bus; +return _machine->main_system_bus.parent_obj; } static void sysbus_register_types(void) diff --git a/include/hw/boards.h b/include/hw/boards.h index 7af940102d..63a4f990ea 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -11,6 +11,7 @@ #include "qemu/module.h" #include "qom/object.h" #include "hw/core/cpu.h" +#include "hw/sysbus.h" #define TYPE_MACHINE_SUFFIX "-machine" @@ -346,7 +347,7 @@ struct MachineState { */ MemoryRegion *ram; DeviceMemoryState *device_memory; -BusState main_system_bus; +SysBusState main_system_bus; ram_addr_t ram_size; ram_addr_t maxram_size; diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h index 6c05f3bfd2..0944be587f 100644 --- a/include/hw/misc/macio/macio.h +++ b/include/hw/misc/macio/macio.h @@ -44,7 +44,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(MacIOBusState, MACIO_BUS) struct MacIOBusState { /*< private >*/ -BusState parent_obj; +SysBusState parent_obj; }; /* MacIO IDE */ diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h index 3564b7b6a2..5bb3b88501 100644 --- a/include/hw/sysbus.h +++ b/include/hw/sysbus.h @@ -11,9 +11,13 @@ #define QDEV_MAX_PIO 32 #define TYPE_SYSTEM_BUS "System" -DECLARE_INSTANCE_CHECKER(BusState, SYSTEM_BUS, - TYPE_SYSTEM_BUS) +OBJECT_DECLARE_SIMPLE_TYPE(SysBusState, SYSTEM_BUS) +struct SysBusState { +/*< private >*/ +BusState parent_obj; +/*< public >*/ +}; #define TYPE_SYS_BUS_DEVICE "sys-bus-device" OBJECT_DECLARE_TYPE(SysBusDevice, SysBusDeviceClass, -- 2.37.3
[PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to inherit from TYPE_MACHINE. This is an inconsistency which can cause undefined behavior such as memory corruption. Change SiFiveEState to inherit from MachineState since it is registered as a machine. Signed-off-by: Bernhard Beschow --- include/hw/riscv/sifive_e.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h index 83604da805..d738745925 100644 --- a/include/hw/riscv/sifive_e.h +++ b/include/hw/riscv/sifive_e.h @@ -22,6 +22,7 @@ #include "hw/riscv/riscv_hart.h" #include "hw/riscv/sifive_cpu.h" #include "hw/gpio/sifive_gpio.h" +#include "hw/boards.h" #define TYPE_RISCV_E_SOC "riscv.sifive.e.soc" #define RISCV_E_SOC(obj) \ @@ -41,7 +42,7 @@ typedef struct SiFiveESoCState { typedef struct SiFiveEState { /*< private >*/ -SysBusDevice parent_obj; +MachineState parent_obj; /*< public >*/ SiFiveESoCState soc; -- 2.37.3
[PATCH 2/9] exec/hwaddr.h: Add missing include
The next commit would not compile w/o the include directive. Signed-off-by: Bernhard Beschow --- include/exec/hwaddr.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/exec/hwaddr.h b/include/exec/hwaddr.h index 8f16d179a8..616255317c 100644 --- a/include/exec/hwaddr.h +++ b/include/exec/hwaddr.h @@ -3,6 +3,7 @@ #ifndef HWADDR_H #define HWADDR_H +#include "qemu/osdep.h" #define HWADDR_BITS 64 /* hwaddr is the type of a physical address (its size can -- 2.37.3
[PATCH 0/9] Deprecate sysbus_get_default() and get_system_memory() et. al
In address-spaces.h it can be read that get_system_memory() and get_system_io() are temporary interfaces which "should only be used temporarily until a proper bus interface is available". This statement certainly extends to the address_space_memory and address_space_io singletons. This series attempts to stop further proliferation of their use by turning TYPE_SYSTEM_BUS into an object-oriented, "proper bus interface" inspired by PCIBus. While at it, also the main_system_bus singleton is turned into an attribute of MachineState. Together, this resolves five singletons in total, making the ownership relations much more obvious which helps comprehension. The series is structured as follows: Patch 1 fixes a memory corruption issue uncovered by running `make check` on the last but one patch of this series. Patches 2 and 3 turn the main_system_bus singleton into an attribute of MachineState which provides an alternative to sysbus_get_default(). Patches 4-7 resolve the address space singletons and deprecate the legacy get_system_memory() et. al functions. Patch 8 attempts to optimize the new implementations of these legacy functions. Testing done: * make check (passes without any issues) * make check-avocado (no new issues seem to be introduced compared to master) Bernhard Beschow (9): hw/riscv/sifive_e: Fix inheritance of SiFiveEState exec/hwaddr.h: Add missing include hw/core/sysbus: Resolve main_system_bus singleton hw/ppc/spapr: Fix code style problems reported by checkpatch exec/address-spaces: Wrap address space singletons into functions target/loongarch/cpu: Remove unneeded include directive hw/sysbus: Introduce dedicated struct SysBusState for TYPE_SYSTEM_BUS softmmu/physmem: Let SysBusState absorb memory region and address space singletons exec/address-spaces: Inline legacy functions accel/hvf/hvf-accel-ops.c| 2 +- accel/kvm/kvm-all.c | 12 +++ hw/alpha/dp264.c | 4 +-- hw/alpha/typhoon.c | 4 +-- hw/arm/smmu-common.c | 4 +-- hw/arm/smmuv3.c | 14 hw/arm/virt.c| 2 +- hw/char/goldfish_tty.c | 4 +-- hw/core/bus.c| 5 ++- hw/core/loader.c | 2 +- hw/core/machine.c| 3 ++ hw/core/sysbus.c | 24 -- hw/dma/pl330.c | 2 +- hw/dma/rc4030.c | 2 +- hw/dma/xlnx-zynq-devcfg.c| 4 +-- hw/dma/xlnx_dpdma.c | 8 ++--- hw/hppa/machine.c| 4 +-- hw/hyperv/hyperv.c | 2 +- hw/hyperv/vmbus.c| 2 +- hw/i386/amd_iommu.c | 18 +- hw/i386/fw_cfg.c | 2 +- hw/i386/intel_iommu.c| 24 +++--- hw/i386/microvm.c| 4 +-- hw/i386/pc.c | 2 +- hw/i386/xen/xen-hvm.c| 4 +-- hw/ide/ahci.c| 2 +- hw/ide/macio.c | 10 +++--- hw/intc/apic.c | 2 +- hw/intc/openpic_kvm.c| 2 +- hw/intc/pnv_xive.c | 6 ++-- hw/intc/pnv_xive2.c | 6 ++-- hw/intc/riscv_aplic.c| 2 +- hw/intc/spapr_xive.c | 2 +- hw/intc/xive.c | 4 +-- hw/intc/xive2.c | 4 +-- hw/mips/jazz.c | 4 +-- hw/misc/lasi.c | 2 +- hw/misc/macio/mac_dbdma.c| 8 ++--- hw/net/ftgmac100.c | 16 - hw/net/i82596.c | 24 +++--- hw/net/imx_fec.c | 22 ++--- hw/net/lasi_i82596.c | 2 +- hw/net/npcm7xx_emc.c | 14 hw/openrisc/boot.c | 2 +- hw/pci-host/dino.c | 6 ++-- hw/pci-host/pnv_phb3.c | 6 ++-- hw/pci-host/pnv_phb3_msi.c | 6 ++-- hw/pci-host/pnv_phb4.c | 10 +++--- hw/pci/pci.c | 2 +- hw/ppc/pnv_psi.c | 2 +- hw/ppc/spapr.c | 4 +-- hw/ppc/spapr_events.c| 2 +- hw/ppc/spapr_hcall.c | 4 +-- hw/ppc/spapr_iommu.c | 4 +-- hw/ppc/spapr_ovec.c | 8 ++--- hw/ppc/spapr_rtas.c | 2 +- hw/remote/iommu.c| 2 +- hw/remote/message.c | 4 +-- hw/remote/proxy-memory-listener.c| 2 +- hw/riscv/boot.c | 6 ++-- hw/riscv/sifive_e.c | 2 +- hw/riscv/sifive_u.c | 2 +- hw/riscv/virt.c | 2 +- hw/s390x/css.c | 16 - hw/s390x/ipl.h | 2 +- hw/s390x/s390-pci-bus.c
Re: [PATCH 0/3] Add a host power device
Hi Jian, On 19/9/22 19:21, Jian Zhang wrote: This patchset adds a host power device and added it into the g220a mahcine. The BMC have a important is to control the power of the host, usually it is nessary in a hardware platform. The BMC(soc) usually had a output pin to control the power of the host, and a input pin to get the power status of the host. The host power device is a generic device to simulate the host power, accept the power control command from the BMC and report the power status. Test on the g220a machine, the host power control command can be simply work. Jian Zhang (3): hw/gpio/aspeed_gpio: Add gpios in/out init hw/misc/host_power: Add a simple host power device hw/arm/aspeed: g220a: Add host-power device "power-good" is just a TYPE_LED object, but it doesn't seem you are really interested in using it. My understanding of your "power-button" is a latching switch. This could be indeed useful. I'd name this model TYPE_LATCHING_SWITCH and put it in hw/misc/latching-switch.c (since it is external to a SoC). It has one input and one output. Naming them is not particularly useful IMHO. The triggering edge should be a property (it might have a default, positive/negative), and the switch state must to be in vmstate for the object to be migratable. ("power-good"/"power-button" is what this particular board choose to use the latch switch input/output for). Do you mind renaming your series accordingly ("latching switch"), and adding the vmstate? Also I'd reorder your series as 2,1,3: - introduce the new device - prepare aspeed_gpio - wire aspeed_g220a Regards, Phil.
Re: [PATCH] tests/qtest: npcm7xx-emc-test: Skip checking MAC
On Mon, Sep 19, 2022 at 5:44 AM Thomas Huth wrote: > On 06/09/2022 18.31, Patrick Venture wrote: > > The register tests walks all the registers to verify they are initially > > 0 when appropriate. However, if the MAC address is set in the register > >space, this should not be checked against 0. > > > > Reviewed-by: Hao Wu > > Change-Id: I02426e39bdab33ceedd42c49d233e8680d4ec058 > > What's that change-id good for? > Oops, sorry about that. I can send out a v2 without it, or during application someone can nicely trim it? :) > > > Signed-off-by: Patrick Venture > > --- > > tests/qtest/npcm7xx_emc-test.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qtest/npcm7xx_emc-test.c > b/tests/qtest/npcm7xx_emc-test.c > > index 7c435ac915..207d8515b7 100644 > > --- a/tests/qtest/npcm7xx_emc-test.c > > +++ b/tests/qtest/npcm7xx_emc-test.c > > @@ -378,7 +378,8 @@ static void test_init(gconstpointer test_data) > > > > #undef CHECK_REG > > > > -for (i = 0; i < NUM_CAMML_REGS; ++i) { > > +/* Skip over the MAC address registers, which is BASE+0 */ > > +for (i = 1; i < NUM_CAMML_REGS; ++i) { > > g_assert_cmpuint(emc_read(qts, mod, REG_CAMM_BASE + i * 2), ==, > >0); > > g_assert_cmpuint(emc_read(qts, mod, REG_CAML_BASE + i * 2), ==, > > Basically ack, but one question: Where should that non-zero MAC address > come > from / when did you hit a problem here? If QEMU is started without any mac > settings at all (like it is done here), the register never contains a > non-zero value, does it? > So, there's a bug in the emc device presently where that value isn't set when it should be. I have that bug fixed, but for whatever reason, probably not enough caffeine, I didn't bundle the two patches together. > > Thomas > >
Re: [PATCH 0/2] target/riscv: improvements to GDB target descriptions
On Wed, Aug 31, 2022 at 6:43 PM Andrew Burgess wrote: > > I was running some GDB tests against QEMU, and noticed some oddities > with the target description QEMU sends, the following two patches > address these issues. > > Thanks, > Andrew > > --- > > Andrew Burgess (2): > target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml > target/riscv: remove fixed numbering from GDB xml feature files Thanks! Applied to riscv-to-apply.next Alistair > > gdb-xml/riscv-32bit-cpu.xml | 6 +- > gdb-xml/riscv-32bit-fpu.xml | 10 +- > gdb-xml/riscv-64bit-cpu.xml | 6 +- > gdb-xml/riscv-64bit-fpu.xml | 10 +- > target/riscv/gdbstub.c | 32 ++-- > 5 files changed, 6 insertions(+), 58 deletions(-) > > -- > 2.25.4 > >
Re: [PATCH v2] disas/riscv.c: rvv: Add disas support for vector instructions
On Fri, Aug 26, 2022 at 1:26 PM Yang Liu wrote: > > Tested with https://github.com/ksco/rvv-decoder-tests > > Expected checkpatch errors for consistency and brevity reasons: > > ERROR: line over 90 characters > ERROR: trailing statements should be on next line > ERROR: braces {} are necessary for all arms of this statement > > Signed-off-by: Yang Liu Acked-by: Alistair Francis Alistair > --- > disas/riscv.c | 1432 - > 1 file changed, 1430 insertions(+), 2 deletions(-) > > diff --git a/disas/riscv.c b/disas/riscv.c > index 7af6afc8fa..719a5c18b8 100644 > --- a/disas/riscv.c > +++ b/disas/riscv.c > @@ -158,6 +158,11 @@ typedef enum { > rv_codec_css_sqsp, > rv_codec_k_bs, > rv_codec_k_rnum, > +rv_codec_v_r, > +rv_codec_v_ldst, > +rv_codec_v_i, > +rv_codec_vsetvli, > +rv_codec_vsetivli, > } rv_codec; > > typedef enum { > @@ -560,6 +565,376 @@ typedef enum { > rv_op_zip = 396, > rv_op_xperm4 = 397, > rv_op_xperm8 = 398, > +rv_op_vle8_v = 399, > +rv_op_vle16_v = 400, > +rv_op_vle32_v = 401, > +rv_op_vle64_v = 402, > +rv_op_vse8_v = 403, > +rv_op_vse16_v = 404, > +rv_op_vse32_v = 405, > +rv_op_vse64_v = 406, > +rv_op_vlm_v = 407, > +rv_op_vsm_v = 408, > +rv_op_vlse8_v = 409, > +rv_op_vlse16_v = 410, > +rv_op_vlse32_v = 411, > +rv_op_vlse64_v = 412, > +rv_op_vsse8_v = 413, > +rv_op_vsse16_v = 414, > +rv_op_vsse32_v = 415, > +rv_op_vsse64_v = 416, > +rv_op_vluxei8_v = 417, > +rv_op_vluxei16_v = 418, > +rv_op_vluxei32_v = 419, > +rv_op_vluxei64_v = 420, > +rv_op_vloxei8_v = 421, > +rv_op_vloxei16_v = 422, > +rv_op_vloxei32_v = 423, > +rv_op_vloxei64_v = 424, > +rv_op_vsuxei8_v = 425, > +rv_op_vsuxei16_v = 426, > +rv_op_vsuxei32_v = 427, > +rv_op_vsuxei64_v = 428, > +rv_op_vsoxei8_v = 429, > +rv_op_vsoxei16_v = 430, > +rv_op_vsoxei32_v = 431, > +rv_op_vsoxei64_v = 432, > +rv_op_vle8ff_v = 433, > +rv_op_vle16ff_v = 434, > +rv_op_vle32ff_v = 435, > +rv_op_vle64ff_v = 436, > +rv_op_vl1re8_v = 437, > +rv_op_vl1re16_v = 438, > +rv_op_vl1re32_v = 439, > +rv_op_vl1re64_v = 440, > +rv_op_vl2re8_v = 441, > +rv_op_vl2re16_v = 442, > +rv_op_vl2re32_v = 443, > +rv_op_vl2re64_v = 444, > +rv_op_vl4re8_v = 445, > +rv_op_vl4re16_v = 446, > +rv_op_vl4re32_v = 447, > +rv_op_vl4re64_v = 448, > +rv_op_vl8re8_v = 449, > +rv_op_vl8re16_v = 450, > +rv_op_vl8re32_v = 451, > +rv_op_vl8re64_v = 452, > +rv_op_vs1r_v = 453, > +rv_op_vs2r_v = 454, > +rv_op_vs4r_v = 455, > +rv_op_vs8r_v = 456, > +rv_op_vadd_vv = 457, > +rv_op_vadd_vx = 458, > +rv_op_vadd_vi = 459, > +rv_op_vsub_vv = 460, > +rv_op_vsub_vx = 461, > +rv_op_vrsub_vx = 462, > +rv_op_vrsub_vi = 463, > +rv_op_vwaddu_vv = 464, > +rv_op_vwaddu_vx = 465, > +rv_op_vwadd_vv = 466, > +rv_op_vwadd_vx = 467, > +rv_op_vwsubu_vv = 468, > +rv_op_vwsubu_vx = 469, > +rv_op_vwsub_vv = 470, > +rv_op_vwsub_vx = 471, > +rv_op_vwaddu_wv = 472, > +rv_op_vwaddu_wx = 473, > +rv_op_vwadd_wv = 474, > +rv_op_vwadd_wx = 475, > +rv_op_vwsubu_wv = 476, > +rv_op_vwsubu_wx = 477, > +rv_op_vwsub_wv = 478, > +rv_op_vwsub_wx = 479, > +rv_op_vadc_vvm = 480, > +rv_op_vadc_vxm = 481, > +rv_op_vadc_vim = 482, > +rv_op_vmadc_vvm = 483, > +rv_op_vmadc_vxm = 484, > +rv_op_vmadc_vim = 485, > +rv_op_vsbc_vvm = 486, > +rv_op_vsbc_vxm = 487, > +rv_op_vmsbc_vvm = 488, > +rv_op_vmsbc_vxm = 489, > +rv_op_vand_vv = 490, > +rv_op_vand_vx = 491, > +rv_op_vand_vi = 492, > +rv_op_vor_vv = 493, > +rv_op_vor_vx = 494, > +rv_op_vor_vi = 495, > +rv_op_vxor_vv = 496, > +rv_op_vxor_vx = 497, > +rv_op_vxor_vi = 498, > +rv_op_vsll_vv = 499, > +rv_op_vsll_vx = 500, > +rv_op_vsll_vi = 501, > +rv_op_vsrl_vv = 502, > +rv_op_vsrl_vx = 503, > +rv_op_vsrl_vi = 504, > +rv_op_vsra_vv = 505, > +rv_op_vsra_vx = 506, > +rv_op_vsra_vi = 507, > +rv_op_vnsrl_wv = 508, > +rv_op_vnsrl_wx = 509, > +rv_op_vnsrl_wi = 510, > +rv_op_vnsra_wv = 511, > +rv_op_vnsra_wx = 512, > +rv_op_vnsra_wi = 513, > +rv_op_vmseq_vv = 514, > +rv_op_vmseq_vx = 515, > +rv_op_vmseq_vi = 516, > +rv_op_vmsne_vv = 517, > +rv_op_vmsne_vx = 518, > +rv_op_vmsne_vi = 519, > +rv_op_vmsltu_vv = 520, > +rv_op_vmsltu_vx = 521, > +rv_op_vmslt_vv = 522, > +rv_op_vmslt_vx = 523, > +rv_op_vmsleu_vv = 524, > +rv_op_vmsleu_vx = 525, > +rv_op_vmsleu_vi = 526, > +rv_op_vmsle_vv = 527, > +rv_op_vmsle_vx = 528, > +rv_op_vmsle_vi = 529, > +rv_op_vmsgtu_vx = 530, > +rv_op_vmsgtu_vi = 531, > +rv_op_vmsgt_vx = 532, > +rv_op_vmsgt_vi = 533, > +rv_op_vminu_vv = 534, > +
Re: [PATCH v14 0/5] Improve PMU support
On Thu, Aug 25, 2022 at 8:22 AM Atish Patra wrote: > > The latest version of the SBI specification includes a Performance Monitoring > Unit(PMU) extension[1] which allows the supervisor to start/stop/configure > various PMU events. The Sscofpmf ('Ss' for Privileged arch and > Supervisor-level > extensions, and 'cofpmf' for Count OverFlow and Privilege Mode Filtering) > extension[2] allows the perf like tool to handle overflow interrupts and > filtering support. > > This series implements remaining PMU infrastructure to support > PMU in virt machine. The first seven patches from the original series > have been merged already. > > This will allow us to add any PMU events in future. > Currently, this series enables the following omu events. > 1. cycle count > 2. instruction count > 3. DTLB load/store miss > 4. ITLB prefetch miss > > The first two are computed using host ticks while last three are counted > during > cpu_tlb_fill. We can do both sampling and count from guest userspace. > This series has been tested on both RV64 and RV32. Both Linux[3] and > Opensbi[4] > patches are required to get the perf working. > > Here is an output of perf stat/report while running hackbench with latest > OpenSBI & Linux kernel. > > Perf stat: > == > [root@fedora-riscv ~]# perf stat -e cycles -e instructions -e > dTLB-load-misses -e dTLB-store-misses -e iTLB-load-misses \ > > perf bench sched messaging -g 1 -l 10 > # Running 'sched/messaging' benchmark: > # 20 sender and receiver processes per group > # 1 groups == 40 processes run > > Total time: 0.265 [sec] > > Performance counter stats for 'perf bench sched messaging -g 1 -l 10': > > 4,167,825,362 cycles > 4,166,609,256 instructions #1.00 insn per cycle > 3,092,026 dTLB-load-misses >258,280 dTLB-store-misses > 2,068,966 iTLB-load-misses > >0.585791767 seconds time elapsed > >0.373802000 seconds user >1.042359000 seconds sys > > Perf record: > > [root@fedora-riscv ~]# perf record -e cycles -e instructions \ > > -e dTLB-load-misses -e dTLB-store-misses -e iTLB-load-misses -c 1 \ > > perf bench sched messaging -g 1 -l 10 > # Running 'sched/messaging' benchmark: > # 20 sender and receiver processes per group > # 1 groups == 40 processes run > > Total time: 1.397 [sec] > [ perf record: Woken up 10 times to write data ] > Check IO/CPU overload! > [ perf record: Captured and wrote 8.211 MB perf.data (214486 samples) ] > > [root@fedora-riscv riscv]# perf report > Available samples > 107K cycles > ◆ > 107K instructions > ▒ > 250 dTLB-load-misses > ▒ > 13 dTLB-store-misses > ▒ > 172 iTLB-load-misses > .. > > Changes from v13->v14: > 1. Added sanity check for the hashtable in pmu.c > > Changes from v12->v13: > 1. Rebased on top of the apply-next. > 2. Addressed comments about space & comment block. > > Changes from v11->v12: > 1. Rebased on top of the apply-next. > 2. Aligned the write function & .min_priv to the previous line. > 3. Fixed the FDT generations for multi-socket scenario. > 4. Dropped interrupt property from the DT. > 5. Generate illegal instruction fault instead of virtual instruction fault >for VS/VU access while mcounteren is not set. > > Changes from v10->v11: > 1. Rebased on top of the master where first 7 patches were already merged. > 2. Removed unnecessary additional check in ctr predicate function. > 3. Removed unnecessary priv version checks in mcountinhibit read/write. > 4. Added Heiko's reviewed-by/tested-by tags. > > Changes from v8->v9: > 1. Added the write_done flags to the vmstate. > 2. Fixed the hpmcounter read access from M-mode. > > Changes from v7->v8: > 1. Removeding ordering constraints for mhpmcounter & mhpmevent. > > Changes from v6->v7: > 1. Fixed all the compilation errors for the usermode. > > Changes from v5->v6: > 1. Fixed compilation issue with PATCH 1. > 2. Addressed other comments. > > Changes from v4->v5: > 1. Rebased on top of the -next with following patches. >- isa extension >- priv 1.12 spec > 2. Addressed all the comments on v4 > 3. Removed additional isa-ext DT node in favor of riscv,isa string update > > Changes from v3->v4: > 1. Removed the dummy events from pmu DT node. > 2. Fixed pmu_avail_counters mask generation. > 3. Added a patch to simplify the predicate function for counters. > > Changes from v2->v3: > 1. Addressed all the comments on PATCH1-4. > 2. Split patch1 into two separate patches. > 3. Added explicit comments to explain the event types in DT node. > 4. Rebased on latest Qemu. > > Changes from v1->v2: > 1. Dropped the ACks from v1 as signficant changes happened after v1. > 2. sscofpmf support. > 3. A generic
Re: [PATCH 2/2] virtio-blk: add zoned storage emulation for zoned devices
On Sat, Sep 10, 2022 at 02:50:57PM +0800, Sam Li wrote: > This patch extends virtio-blk emulation to handle zoned device commands > by calling the new block layer APIs to perform zoned device I/O on > behalf of the guest. It supports Report Zone, four zone oparations (open, > close, finish, reset), and Append Zone. > > The VIRTIO_BLK_F_ZONED feature bit will only be set if the host does > support zoned block devices. Regular block devices(conventional zones) > will not be set. > > The guest os having zoned device support can use blkzone(8) to test those > commands. Furthermore, using zonefs to test zone append write is also > supported. > > Signed-off-by: Sam Li > --- > hw/block/virtio-blk.c | 326 ++ > 1 file changed, 326 insertions(+) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index e9ba752f6b..3ef74c01db 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -46,6 +46,8 @@ static const VirtIOFeature feature_sizes[] = { > .end = endof(struct virtio_blk_config, discard_sector_alignment)}, > {.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES, > .end = endof(struct virtio_blk_config, write_zeroes_may_unmap)}, > +{.flags = 1ULL << VIRTIO_BLK_F_ZONED, > + .end = endof(struct virtio_blk_config, zoned)}, > {} > }; > > @@ -614,6 +616,273 @@ err: > return err_status; > } > > +typedef struct ZoneCmdData { > +VirtIOBlockReq *req; > +union { > +struct { > +unsigned int nr_zones; > +BlockZoneDescriptor *zones; > +} ZoneReportData; > +struct { > +int64_t append_sector; > +} ZoneAppendData; Field names should be lowercase: struct { unsigned int nr_zones; BlockZoneDescriptor *zones; } zone_report_data; struct { int64_t append_sector; } zone_append_data; > +}; > +} ZoneCmdData; > + > +/* > + * check zone_model: error checking before issuing requests. If all checks Maybe rename it to check_zoned_request()? It does more than check the model. > + * passed, return true. > + * append: true if only zone append request issued. > + */ > +static bool check_zone_model(VirtIOBlock *s, int64_t sector, int64_t > nr_sector, > + bool append, uint8_t *status) { > +BlockDriverState *bs = blk_bs(s->blk); > +BlockZoneDescriptor *zone = >bl.zones[sector / bs->bl.zone_sectors]; Inputs from the guest driver are untrusted and must be validated before using them. sector could have any value here, including invalid values. Please check that sector is less than the device capacity and also that it is positive. > +int64_t max_append_sector = bs->bl.max_append_sectors; > + > +if (!virtio_has_feature(s->host_features, VIRTIO_BLK_F_ZONED)) { > +*status = VIRTIO_BLK_S_UNSUPP; > +return false; > +} > + > +if (zone->cond == BLK_ZS_OFFLINE) { > +*status = VIRTIO_BLK_S_ZONE_INVALID_CMD; > +return false; > +} > + > +if (append) { > +if ((zone->type != BLK_ZT_SWR) || (zone->cond == BLK_ZS_RDONLY) || > +(sector + nr_sector > (*(zone + 1)).start)) { > +/* the end sector of the request exceeds to next zone */ > +*status = VIRTIO_BLK_S_ZONE_INVALID_CMD; > +return false; > +} > + > +if (nr_sector > max_append_sector) { > +if (max_append_sector == 0) { > +*status = VIRTIO_BLK_S_UNSUPP; > +} else { > +*status = VIRTIO_BLK_S_ZONE_INVALID_CMD; > +} > +return false; > +} > +} > +return true; > +} > + > +static void virtio_blk_zone_report_complete(void *opaque, int ret) > +{ > +ZoneCmdData *data = opaque; > +VirtIOBlockReq *req = data->req; > +VirtIOBlock *s = req->dev; > +VirtIODevice *vdev = VIRTIO_DEVICE(req->dev); > +struct iovec *in_iov = req->elem.in_sg; > +unsigned in_num = req->elem.in_num; > +int64_t zrp_size, nz, n, j = 0; > +int8_t err_status = VIRTIO_BLK_S_OK; > + > +nz = data->ZoneReportData.nr_zones; > +struct virtio_blk_zone_report zrp_hdr = (struct virtio_blk_zone_report) { > +.nr_zones = cpu_to_le64(nz), > +}; > + > +zrp_size = sizeof(struct virtio_blk_zone_report) > + + sizeof(struct virtio_blk_zone_descriptor) * nz; > +n = iov_from_buf(in_iov, in_num, 0, _hdr, sizeof(zrp_hdr)); > +if (n != sizeof(zrp_hdr)) { > +virtio_error(vdev, "Driver provided intput buffer that is too > small!"); > +err_status = VIRTIO_BLK_S_ZONE_INVALID_CMD; > +goto out; > +} > + > +for (size_t i = sizeof(zrp_hdr); i < zrp_size; i += sizeof(struct > virtio_blk_zone_descriptor), ++j) { > +struct virtio_blk_zone_descriptor desc = > +(struct virtio_blk_zone_descriptor) { > +.z_start = >
Re: Fast usermode networking with QEMU
Hi Anders, On Mon, 23 May 2022 14:51:17 -0600 "Anders Pitman" wrote: > I came across this blog post[0] concerning passt, which is an > alternative usermode networking implementation for QEMU. ...and I just came across your email, entirely by chance. I'm not actively monitoring this list most of the time. > I'm working on a project that uses QEMU on Windows hosts running > Linux guests. I'm trying to get faster usermode networking than is > available with libslirp. My performance target is 200Mbps even on > older or less powerful hardware, such as Celeron mini PCs. Currently > I'm seeing 15-30MBps with libslirp. > > It appears that passt doesn't currently support Windows, correct? Is > there a guess as to how difficult that might be to implement? If the > speedup is significant, I would be interested in taking a crack at > adding Windows support. I gave some quick comments about the challenges I see in the perspective of a FreeBSD port at: https://bugs.passt.top/show_bug.cgi?id=6#c2 and I suppose a Windows port, by the way tracked at: https://bugs.passt.top/show_bug.cgi?id=8 could face similar challenges. But from a quick browsing of the Winsock reference documentation months ago, it actually looked easier because there seemed to be a description of an equivalent for every bit of TCP socket information we're fetching on Linux via TCP_INFO socket option. I can try to remember/double check and give more details if it helps, but if you're familiar with Windows development I suppose I wouldn't add much value. :) -- Stefano
Re: [PATCH] vfio/common: Fix vfio_iommu_type1_info use after free
On Thu, Sep 15, 2022 at 11:18:27AM -0600, Alex Williamson wrote: > External email: Use caution opening links or attachments > > > On error, vfio_get_iommu_info() frees and clears *info, but > vfio_connect_container() continues to use the pointer regardless > of the return value. Restructure the code such that a failure > of this function triggers an error and clean up the remainder of > the function, including updating an outdated comment that had > drifted from its relevant line of code and using host page size > for a default for better compatibility on non-4KB systems. > > Reported-by: Nicolin Chen > Link: https://lore.kernel.org/all/20220910004245.2878-1-nicol...@nvidia.com/ > Signed-off-by: Alex Williamson Reviewed-by: Nicolin Chen Tested-by: Nicolin Chen Thanks!
[PULL 1/2] Hexagon (target/hexagon) remove unused encodings
Remove encodings guarded by ifdef that is not defined Signed-off-by: Taylor Simpson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20220606222327.7682-4-tsimp...@quicinc.com> --- target/hexagon/imported/encode_pp.def | 23 --- 1 file changed, 23 deletions(-) diff --git a/target/hexagon/imported/encode_pp.def b/target/hexagon/imported/encode_pp.def index 939c6fc55f..d71c04cd30 100644 --- a/target/hexagon/imported/encode_pp.def +++ b/target/hexagon/imported/encode_pp.def @@ -944,13 +944,6 @@ MPY_ENC(F2_dfmpyfix, "1000","d","0","0","1","0","11") MPY_ENC(F2_dfmin,"1000","d","0","0","1","1","11") MPY_ENC(F2_dfmax,"1000","d","0","1","0","0","11") MPY_ENC(F2_dfmpyll, "1000","d","0","1","0","1","11") -#ifdef ADD_DP_OPS -MPY_ENC(F2_dfdivcheat, "1000","d","0","0","0","1","00") - -MPY_ENC(F2_dffixupn, "1000","d","0","1","0","1","11") -MPY_ENC(F2_dffixupd, "1000","d","0","1","1","0","11") -MPY_ENC(F2_dfrecipa, "1000","d","0","1","1","1","ee") -#endif MPY_ENC(M7_dcmpyrw, "1000","d","0","0","0","1","10") MPY_ENC(M7_dcmpyrwc, "1000","d","0","0","1","1","10") @@ -1024,15 +1017,6 @@ MPY_ENC(M5_vdmacbsu, "1010","x","0","1","0","0","01") MPY_ENC(F2_dfmpylh, "1010","x","0","0","0","0","11") MPY_ENC(F2_dfmpyhh, "1010","x","0","0","0","1","11") -#ifdef ADD_DP_OPS -MPY_ENC(F2_dfmpyhh, "1010","x","0","0","1","0","11") -MPY_ENC(F2_dffma,"1010","x","0","0","0","0","11") -MPY_ENC(F2_dffms,"1010","x","0","0","0","1","11") - -MPY_ENC(F2_dffma_lib,"1010","x","0","0","1","0","11") -MPY_ENC(F2_dffms_lib,"1010","x","0","0","1","1","11") -MPY_ENC(F2_dffma_sc, "1010","x","0","1","1","1","uu") -#endif MPY_ENC(M7_dcmpyrw_acc, "1010","x","0","0","0","1","10") @@ -1547,15 +1531,8 @@ SH2_RR_ENC(F2_conv_df2d, "","111","0","0 00","d") SH2_RR_ENC(F2_conv_df2ud, "","111","0","0 01","d") SH2_RR_ENC(F2_conv_ud2df, "","111","0","0 10","d") SH2_RR_ENC(F2_conv_d2df, "","111","0","0 11","d") -#ifdef ADD_DP_OPS -SH2_RR_ENC(F2_dffixupr, "","111","0","1 00","d") -SH2_RR_ENC(F2_dfsqrtcheat,"","111","0","1 01","d") -#endif SH2_RR_ENC(F2_conv_df2d_chop, "","111","0","1 10","d") SH2_RR_ENC(F2_conv_df2ud_chop,"","111","0","1 11","d") -#ifdef ADD_DP_OPS -SH2_RR_ENC(F2_dfinvsqrta, "","111","1","0 ee","d") -#endif -- 2.17.1
[PULL 0/2] Hexagon target update
The following changes since commit d29201ff34a135cdfc197f4413c1c5047e4f58bb: Merge tag 'pull-hmp-20220915a' of https://gitlab.com/dagrh/qemu into staging (2022-09-17 10:31:11 -0400) are available in the Git repository at: https://github.com/quic/qemu tags/pull-hex-20220919 for you to fetch changes up to ee42af726b9aba8245022fd4b7350a12acd3: Hexagon (tests/tcg/hexagon): add fmin/fmax tests for signed zero (2022-09-19 11:55:23 -0700) Hexagon target update remove unused encodings add fmin/fmax tests for signed zero Matheus Tavares Bernardino (1): Hexagon (tests/tcg/hexagon): add fmin/fmax tests for signed zero Taylor Simpson (1): Hexagon (target/hexagon) remove unused encodings tests/tcg/hexagon/usr.c | 10 ++ target/hexagon/imported/encode_pp.def | 23 --- 2 files changed, 10 insertions(+), 23 deletions(-)
[PULL 2/2] Hexagon (tests/tcg/hexagon): add fmin/fmax tests for signed zero
From: Matheus Tavares Bernardino Signed-off-by: Matheus Tavares Bernardino Signed-off-by: Taylor Simpson Reviewed-by: Taylor Simpson Tested-by: Taylor Simpson --- tests/tcg/hexagon/usr.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/tests/tcg/hexagon/usr.c b/tests/tcg/hexagon/usr.c index a531511cec..fb4514989c 100644 --- a/tests/tcg/hexagon/usr.c +++ b/tests/tcg/hexagon/usr.c @@ -86,6 +86,7 @@ const uint32_t SF_QNaN_neg = 0xffc0; const uint32_t SF_SNaN_neg = 0xffb0; const uint32_t SF_HEX_NaN = 0x; const uint32_t SF_zero = 0x; +const uint32_t SF_zero_neg = 0x8000; const uint32_t SF_one = 0x3f80; const uint32_t SF_one_recip =0x3f7f0001; /* 0.9960... */ const uint32_t SF_one_invsqrta = 0x3f7f; /* 0.99609375 */ @@ -100,6 +101,7 @@ const uint64_t DF_QNaN_neg = 0xfff8ULL; const uint64_t DF_SNaN_neg = 0xfff7ULL; const uint64_t DF_HEX_NaN = 0xULL; const uint64_t DF_zero = 0xULL; +const uint64_t DF_zero_neg = 0x8000ULL; const uint64_t DF_any = 0x3f80ULL; const uint64_t DF_one = 0x3ff0ULL; const uint64_t DF_one_hh = 0x3ff001ff8000ULL; /* 1.00048... */ @@ -933,6 +935,8 @@ int main() TEST_R_OP_RR(sfmin, SF_QNaN, SF_one, SF_one, USR_CLEAR); TEST_R_OP_RR(sfmin, SF_SNaN, SF_QNaN,SF_HEX_NaN, USR_FPINVF); TEST_R_OP_RR(sfmin, SF_QNaN, SF_SNaN,SF_HEX_NaN, USR_FPINVF); +TEST_R_OP_RR(sfmin, SF_zero, SF_zero_neg,SF_zero_neg, USR_CLEAR); +TEST_R_OP_RR(sfmin, SF_zero_neg, SF_zero,SF_zero_neg, USR_CLEAR); TEST_R_OP_RR(sfmax, SF_one, SF_small_neg, SF_one, USR_CLEAR); TEST_R_OP_RR(sfmax, SF_one, SF_SNaN,SF_one, USR_FPINVF); @@ -941,6 +945,8 @@ int main() TEST_R_OP_RR(sfmax, SF_QNaN, SF_one, SF_one, USR_CLEAR); TEST_R_OP_RR(sfmax, SF_SNaN, SF_QNaN,SF_HEX_NaN, USR_FPINVF); TEST_R_OP_RR(sfmax, SF_QNaN, SF_SNaN,SF_HEX_NaN, USR_FPINVF); +TEST_R_OP_RR(sfmax, SF_zero, SF_zero_neg,SF_zero, USR_CLEAR); +TEST_R_OP_RR(sfmax, SF_zero_neg, SF_zero,SF_zero, USR_CLEAR); TEST_R_OP_RR(sfadd, SF_one, SF_QNaN,SF_HEX_NaN, USR_CLEAR); TEST_R_OP_RR(sfadd, SF_one, SF_SNaN,SF_HEX_NaN, USR_FPINVF); @@ -1003,6 +1009,8 @@ int main() TEST_P_OP_PP(dfmin, DF_QNaN, DF_any, DF_any,USR_CLEAR); TEST_P_OP_PP(dfmin, DF_SNaN, DF_QNaN, DF_HEX_NaN, USR_FPINVF); TEST_P_OP_PP(dfmin, DF_QNaN, DF_SNaN, DF_HEX_NaN, USR_FPINVF); +TEST_P_OP_PP(dfmin, DF_zero, DF_zero_neg, DF_zero_neg, USR_CLEAR); +TEST_P_OP_PP(dfmin, DF_zero_neg, DF_zero, DF_zero_neg, USR_CLEAR); TEST_P_OP_PP(dfmax, DF_any,DF_small_neg,DF_any,USR_CLEAR); TEST_P_OP_PP(dfmax, DF_any,DF_SNaN, DF_any, USR_FPINVF); @@ -1011,6 +1019,8 @@ int main() TEST_P_OP_PP(dfmax, DF_QNaN, DF_any, DF_any,USR_CLEAR); TEST_P_OP_PP(dfmax, DF_SNaN, DF_QNaN, DF_HEX_NaN, USR_FPINVF); TEST_P_OP_PP(dfmax, DF_QNaN, DF_SNaN, DF_HEX_NaN, USR_FPINVF); +TEST_P_OP_PP(dfmax, DF_zero, DF_zero_neg, DF_zero, USR_CLEAR); +TEST_P_OP_PP(dfmax, DF_zero_neg, DF_zero, DF_zero, USR_CLEAR); TEST_XP_OP_PP(dfmpyhh, DF_one, DF_one, DF_one, DF_one_hh, USR_CLEAR); TEST_XP_OP_PP(dfmpyhh, DF_zero, DF_any, DF_QNaN, DF_HEX_NaN, USR_CLEAR); -- 2.17.1
Re: [PATCH 1/2] include: import virtio_blk headers from linux with zoned device support
On Sat, Sep 10, 2022 at 02:50:56PM +0800, Sam Li wrote: > Add file from Dmitry's "virtio-blk:add support for zoned block devices" > linux patch using scripts/update-linux-headers.sh. There is a link for > more information: https://github.com/dmitry-fomichev/virtblk-zbd Hi Sam, Linux headers are imported into QEMU using scripts/update-linux-headers.sh. Did you import the header using this script? If yes, please mention it in the commit description. If not, please do so in the next revision. Thanks, Stefan > > Signed-off-by: Sam Li > --- > include/standard-headers/linux/virtio_blk.h | 109 > 1 file changed, 109 insertions(+) > > diff --git a/include/standard-headers/linux/virtio_blk.h > b/include/standard-headers/linux/virtio_blk.h > index 2dcc90826a..490bd21c76 100644 > --- a/include/standard-headers/linux/virtio_blk.h > +++ b/include/standard-headers/linux/virtio_blk.h > @@ -40,6 +40,7 @@ > #define VIRTIO_BLK_F_MQ 12 /* support more than one vq */ > #define VIRTIO_BLK_F_DISCARD 13 /* DISCARD is supported */ > #define VIRTIO_BLK_F_WRITE_ZEROES14 /* WRITE ZEROES is supported */ > +#define VIRTIO_BLK_F_ZONED 17 /* Zoned block device */ > > /* Legacy feature bits */ > #ifndef VIRTIO_BLK_NO_LEGACY > @@ -119,6 +120,20 @@ struct virtio_blk_config { > uint8_t write_zeroes_may_unmap; > > uint8_t unused1[3]; > + > + /* Secure erase fields that are defined in the virtio spec */ > + uint8_t sec_erase[12]; > + > + /* Zoned block device characteristics (if VIRTIO_BLK_F_ZONED) */ > + struct virtio_blk_zoned_characteristics { > + __virtio32 zone_sectors; > + __virtio32 max_open_zones; > + __virtio32 max_active_zones; > + __virtio32 max_append_sectors; > + __virtio32 write_granularity; > + uint8_t model; > + uint8_t unused2[3]; > + } zoned; > } QEMU_PACKED; > > /* > @@ -153,6 +168,27 @@ struct virtio_blk_config { > /* Write zeroes command */ > #define VIRTIO_BLK_T_WRITE_ZEROES13 > > +/* Zone append command */ > +#define VIRTIO_BLK_T_ZONE_APPEND15 > + > +/* Report zones command */ > +#define VIRTIO_BLK_T_ZONE_REPORT16 > + > +/* Open zone command */ > +#define VIRTIO_BLK_T_ZONE_OPEN 18 > + > +/* Close zone command */ > +#define VIRTIO_BLK_T_ZONE_CLOSE 20 > + > +/* Finish zone command */ > +#define VIRTIO_BLK_T_ZONE_FINISH22 > + > +/* Reset zone command */ > +#define VIRTIO_BLK_T_ZONE_RESET 24 > + > +/* Reset All zones command */ > +#define VIRTIO_BLK_T_ZONE_RESET_ALL 26 > + > #ifndef VIRTIO_BLK_NO_LEGACY > /* Barrier before this op. */ > #define VIRTIO_BLK_T_BARRIER 0x8000 > @@ -172,6 +208,72 @@ struct virtio_blk_outhdr { > __virtio64 sector; > }; > > +/* > + * Supported zoned device models. > + */ > + > +/* Regular block device */ > +#define VIRTIO_BLK_Z_NONE 0 > +/* Host-managed zoned device */ > +#define VIRTIO_BLK_Z_HM1 > +/* Host-aware zoned device */ > +#define VIRTIO_BLK_Z_HA2 > + > +/* > + * Zone descriptor. A part of VIRTIO_BLK_T_ZONE_REPORT command reply. > + */ > +struct virtio_blk_zone_descriptor { > + /* Zone capacity */ > + __virtio64 z_cap; > + /* The starting sector of the zone */ > + __virtio64 z_start; > + /* Zone write pointer position in sectors */ > + __virtio64 z_wp; > + /* Zone type */ > + uint8_t z_type; > + /* Zone state */ > + uint8_t z_state; > + uint8_t reserved[38]; > +}; > + > +struct virtio_blk_zone_report { > + __virtio64 nr_zones; > + uint8_t reserved[56]; > + struct virtio_blk_zone_descriptor zones[]; > +}; > + > +/* > + * Supported zone types. > + */ > + > +/* Conventional zone */ > +#define VIRTIO_BLK_ZT_CONV 1 > +/* Sequential Write Required zone */ > +#define VIRTIO_BLK_ZT_SWR 2 > +/* Sequential Write Preferred zone */ > +#define VIRTIO_BLK_ZT_SWP 3 > + > +/* > + * Zone states that are available for zones of all types. > + */ > + > +/* Not a write pointer (conventional zones only) */ > +#define VIRTIO_BLK_ZS_NOT_WP 0 > +/* Empty */ > +#define VIRTIO_BLK_ZS_EMPTY1 > +/* Implicitly Open */ > +#define VIRTIO_BLK_ZS_IOPEN2 > +/* Explicitly Open */ > +#define VIRTIO_BLK_ZS_EOPEN3 > +/* Closed */ > +#define VIRTIO_BLK_ZS_CLOSED 4 > +/* Read-Only */ > +#define VIRTIO_BLK_ZS_RDONLY 13 > +/* Full */ > +#define VIRTIO_BLK_ZS_FULL 14 > +/* Offline */ > +#define VIRTIO_BLK_ZS_OFFLINE 15 > + > /* Unmap this range (only valid for write zeroes command) */ > #define VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP 0x0001 > > @@ -198,4 +300,11 @@ struct virtio_scsi_inhdr { > #define VIRTIO_BLK_S_OK 0 > #define VIRTIO_BLK_S_IOERR 1 > #define VIRTIO_BLK_S_UNSUPP 2 > + > +/* Error codes that are specific to zoned block devices */ > +#define VIRTIO_BLK_S_ZONE_INVALID_CMD 3 > +#define
[PATCH] virtio: add VIRTQUEUE_ERROR QAPI event
For now we only log the vhost device error, when virtqueue is actually stopped. Let's add a QAPI event, which makes possible: - collect statistics of such errors - make immediate actions: take coredums or do some other debugging The event could be reused for some other virtqueue problems (not only for vhost devices) in future. For this it gets a generic name and structure. Signed-off-by: Vladimir Sementsov-Ogievskiy --- hw/virtio/vhost.c | 12 +--- qapi/qdev.json| 25 + 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index f758f177bb..caa81f2ace 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -15,6 +15,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qapi/qapi-events-qdev.h" #include "hw/virtio/vhost.h" #include "qemu/atomic.h" #include "qemu/range.h" @@ -1287,11 +1288,16 @@ static void vhost_virtqueue_error_notifier(EventNotifier *n) struct vhost_virtqueue *vq = container_of(n, struct vhost_virtqueue, error_notifier); struct vhost_dev *dev = vq->dev; -int index = vq - dev->vqs; if (event_notifier_test_and_clear(n) && dev->vdev) { -VHOST_OPS_DEBUG(-EINVAL, "vhost vring error in virtqueue %d", -dev->vq_index + index); +int ind = vq - dev->vqs + dev->vq_index; +DeviceState *ds = >vdev->parent_obj; + +VHOST_OPS_DEBUG(-EINVAL, "vhost vring error in virtqueue %d", ind); +qapi_event_send_virtqueue_error(!!ds->id, ds->id, ds->canonical_path, +ind, VIRTQUEUE_ERROR_VHOST_VRING_ERR, +"vhost reported failure through vring " +"error fd"); } } diff --git a/qapi/qdev.json b/qapi/qdev.json index 2708fb4e99..b7c2669c2c 100644 --- a/qapi/qdev.json +++ b/qapi/qdev.json @@ -158,3 +158,28 @@ ## { 'event': 'DEVICE_UNPLUG_GUEST_ERROR', 'data': { '*device': 'str', 'path': 'str' } } + +## +# @VirtqueueError: +# +# Since: 7.2 +## +{ 'enum': 'VirtqueueError', + 'data': [ 'vhost-vring-err' ] } + +## +# @VIRTQUEUE_ERROR: +# +# Emitted when a device virtqueue fails in runtime. +# +# @device: the device's ID if it has one +# @path: the device's QOM path +# @virtqueue: virtqueue index +# @error: error identifier +# @description: human readable description +# +# Since: 7.2 +## +{ 'event': 'VIRTQUEUE_ERROR', + 'data': { '*device': 'str', 'path': 'str', 'virtqueue': 'int', +'error': 'VirtqueueError', 'description': 'str'} } -- 2.25.1
Re: [PATCH v3] audio: Add sndio backend
On 9/9/2022 2:12 AM, Volker Rümelin wrote: Am 07.09.22 um 15:23 schrieb Alexandre Ratchov: sndio is the native API used by OpenBSD, although it has been ported to other *BSD's and Linux (packages for Ubuntu, Debian, Void, Arch, etc.). Signed-off-by: Brad Smith Signed-off-by: Alexandre Ratchov --- References to the previous patch versions and related discussions are here: https://marc.info/?l=qemu-devel=163973393011543 (v2) https://marc.info/?l=qemu-devel=163626248712444 (initial patch) Here are the changes between v2 and v3 of this patch: - fixed of typos in file-names in MAINTAINERS - added Gerd Hoffmann to the M: entry in MAINTAINERS - added missin S: entry in MAINTAINERS - removed unused #include "qemu-common.h" - bumped "Since:" version to 7.2 in qapi/audio.json - regenerated scripts/meson-buildoptions.sh - implement buffer_get_free() method, introduced by commit 9833438ef624155de879d4ed57ecfcd3464a0bbe audio: restore mixing-engine playback buffer size Running "make update-buildoptions" triggered unrelated changes of scripts/meson-buildoptions.sh, that I removed from the commit as they are not related to sndio. Tested on OpenBSD, still works as expected :-) Regards, Alexandre MAINTAINERS | 7 + audio/audio.c | 1 + audio/audio_template.h | 2 + audio/meson.build | 1 + audio/sndioaudio.c | 565 ++ meson.build | 9 +- meson_options.txt | 4 +- qapi/audio.json | 25 +- qemu-options.hx | 16 + scripts/meson-buildoptions.sh | 7 +- 10 files changed, 632 insertions(+), 5 deletions(-) create mode 100644 audio/sndioaudio.c Tested again on Linux. Reviewed-by: Volker Rümelin Tested-by: Volker Rümelin ping.
[RFC PATCH] libvduse: Do not truncate terminating NUL character with strncpy()
GCC 8 added a -Wstringop-truncation warning: The -Wstringop-truncation warning added in GCC 8.0 via r254630 for bug 81117 is specifically intended to highlight likely unintended uses of the strncpy function that truncate the terminating NUL character from the source string. Here the next line indeed unconditionally zeroes the last byte, so we can call strncpy() on the buffer size less the last byte. This fixes when using gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0: [42/666] Compiling C object subprojects/libvduse/libvduse.a.p/libvduse.c.o FAILED: subprojects/libvduse/libvduse.a.p/libvduse.c.o cc -m64 -mcx16 -Isubprojects/libvduse/libvduse.a.p -Isubprojects/libvduse -I../../subprojects/libvduse [...] -o subprojects/libvduse/libvduse.a.p/libvduse.c.o -c ../../subprojects/libvduse/libvduse.c In file included from /usr/include/string.h:495, from ../../subprojects/libvduse/libvduse.c:24: In function ‘strncpy’, inlined from ‘vduse_dev_create’ at ../../subprojects/libvduse/libvduse.c:1312:5: /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 256 equals destination size [-Werror=stringop-truncation] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~ cc1: all warnings being treated as errors ninja: build stopped: cannot make progress due to previous errors. Fixes: d9cf16c0be ("libvduse: Replace strcpy() with strncpy()") Signed-off-by: Philippe Mathieu-Daudé --- Cc: Xie Yongji Cc: Markus Armbruster Cc: Kevin Wolf RFC: Any better idea? We can't use strpadcpy() because libvduse doesn't depend on QEMU. --- subprojects/libvduse/libvduse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c index 1a5981445c..e460780ce3 100644 --- a/subprojects/libvduse/libvduse.c +++ b/subprojects/libvduse/libvduse.c @@ -1309,7 +1309,7 @@ VduseDev *vduse_dev_create(const char *name, uint32_t device_id, goto err_dev; } -strncpy(dev_config->name, name, VDUSE_NAME_MAX); +strncpy(dev_config->name, name, VDUSE_NAME_MAX - 1); dev_config->name[VDUSE_NAME_MAX - 1] = '\0'; dev_config->device_id = device_id; dev_config->vendor_id = vendor_id; -- 2.37.3
Re: [PATCH] block/qcow2-bitmap: Add missing cast to silent GCC error
On 9/19/22 21:27, Philippe Mathieu-Daudé wrote: Commit d1258dd0c8 ("qcow2: autoloading dirty bitmaps") added the set_readonly_helper() GFunc handler, correctly casting the gpointer user_data in both the g_slist_foreach() caller and the handler. Few commits later (commit 1b6b0562db), the handler is reused in qcow2_reopen_bitmaps_rw() but missing the gpointer cast, resulting in the following error when using Homebrew GCC 12.2.0: [2/658] Compiling C object libblock.fa.p/block_qcow2-bitmap.c.o ../../block/qcow2-bitmap.c: In function 'qcow2_reopen_bitmaps_rw': ../../block/qcow2-bitmap.c:1211:60: error: incompatible type for argument 3 of 'g_slist_foreach' 1211 | g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false); |^ || |_Bool In file included from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gmain.h:26, from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/giochannel.h:33, from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib.h:54, from /Users/philmd/source/qemu/include/glib-compat.h:32, from /Users/philmd/source/qemu/include/qemu/osdep.h:144, from ../../block/qcow2-bitmap.c:28: /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gslist.h:127:61: note: expected 'gpointer' {aka 'void *'} but argument is of type '_Bool' 127 | gpointer user_data); | ~~^ At top level: FAILED: libblock.fa.p/block_qcow2-bitmap.c.o Fix by adding the missing gpointer cast. Fixes: 1b6b0562db ("qcow2: support .bdrv_reopen_bitmaps_rw") Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Vladimir Sementsov-Ogievskiy Thanks for fixing! Seems correct for it to go with trivial patches. -- Best regards, Vladimir
Re: [PATCH v8 1/8] mm/memfd: Introduce userspace inaccessible memfd
+Will, Marc and Fuad (apologies if I missed other pKVM folks) On Mon, Sep 19, 2022, David Hildenbrand wrote: > On 15.09.22 16:29, Chao Peng wrote: > > From: "Kirill A. Shutemov" > > > > KVM can use memfd-provided memory for guest memory. For normal userspace > > accessible memory, KVM userspace (e.g. QEMU) mmaps the memfd into its > > virtual address space and then tells KVM to use the virtual address to > > setup the mapping in the secondary page table (e.g. EPT). > > > > With confidential computing technologies like Intel TDX, the > > memfd-provided memory may be encrypted with special key for special > > software domain (e.g. KVM guest) and is not expected to be directly > > accessed by userspace. Precisely, userspace access to such encrypted > > memory may lead to host crash so it should be prevented. > > Initially my thaught was that this whole inaccessible thing is TDX specific > and there is no need to force that on other mechanisms. That's why I > suggested to not expose this to user space but handle the notifier > requirements internally. > > IIUC now, protected KVM has similar demands. Either access (read/write) of > guest RAM would result in a fault and possibly crash the hypervisor (at > least not the whole machine IIUC). Yep. The missing piece for pKVM is the ability to convert from shared to private while preserving the contents, e.g. to hand off a large buffer (hundreds of MiB) for processing in the protected VM. Thoughts on this at the bottom. > > This patch introduces userspace inaccessible memfd (created with > > MFD_INACCESSIBLE). Its memory is inaccessible from userspace through > > ordinary MMU access (e.g. read/write/mmap) but can be accessed via > > in-kernel interface so KVM can directly interact with core-mm without > > the need to map the memory into KVM userspace. > > With secretmem we decided to not add such "concept switch" flags and instead > use a dedicated syscall. > I have no personal preference whatsoever between a flag and a dedicated syscall, but a dedicated syscall does seem like it would give the kernel a bit more flexibility. > What about memfd_inaccessible()? Especially, sealing and hugetlb are not > even supported and it might take a while to support either. Don't know about sealing, but hugetlb support for "inaccessible" memory needs to come sooner than later. "inaccessible" in quotes because we might want to choose a less binary name, e.g. "restricted"?. Regarding pKVM's use case, with the shim approach I believe this can be done by allowing userspace mmap() the "hidden" memfd, but with a ton of restrictions piled on top. My first thought was to make the uAPI a set of KVM ioctls so that KVM could tightly tightly control usage without taking on too much complexity in the kernel, but working through things, routing the behavior through the shim itself might not be all that horrific. IIRC, we discarded the idea of allowing userspace to map the "private" fd because things got too complex, but with the shim it doesn't seem _that_ bad. E.g. on the memfd side: 1. The entire memfd must be mapped, and at most one mapping is allowed, i.e. mapping is all or nothing. 2. Acquiring a reference via get_pfn() is disallowed if there's a mapping for the restricted memfd. 3. Add notifier hooks to allow downstream users to further restrict things. 4. Disallow splitting VMAs, e.g. to force userspace to munmap() everything in one shot. 5. Require that there are no outstanding references at munmap(). Or if this can't be guaranteed by userspace, maybe add some way for userspace to wait until it's ok to convert to private? E.g. so that get_pfn() doesn't need to do an expensive check every time. static int memfd_restricted_mmap(struct file *file, struct vm_area_struct *vma) { if (vma->vm_pgoff) return -EINVAL; if ((vma->vm_end - vma->vm_start) != ) return -EINVAL; mutex_lock(>lock); if (data->has_mapping) { r = -EINVAL; goto err; } list_for_each_entry(notifier, >notifiers, list) { r = notifier->ops->mmap_start(notifier, ...); if (r) goto abort; } notifier->ops->mmap_end(notifier, ...); mutex_unlock(>lock); return 0; abort: list_for_each_entry_continue_reverse(notifier >notifiers, list) notifier->ops->mmap_abort(notifier, ...); err: mutex_unlock(>lock); return r; } static void memfd_restricted_close(struct vm_area_struct *vma) { mutex_lock(...); /* * Destroy the memfd and disable all future accesses if there are * outstanding refcounts (or other unsatisfied restrictions?). */ if ( || ???) memfd_restricted_destroy(...); else data->has_mapping = false;
[PATCH] block/qcow2-bitmap: Add missing cast to silent GCC error
Commit d1258dd0c8 ("qcow2: autoloading dirty bitmaps") added the set_readonly_helper() GFunc handler, correctly casting the gpointer user_data in both the g_slist_foreach() caller and the handler. Few commits later (commit 1b6b0562db), the handler is reused in qcow2_reopen_bitmaps_rw() but missing the gpointer cast, resulting in the following error when using Homebrew GCC 12.2.0: [2/658] Compiling C object libblock.fa.p/block_qcow2-bitmap.c.o ../../block/qcow2-bitmap.c: In function 'qcow2_reopen_bitmaps_rw': ../../block/qcow2-bitmap.c:1211:60: error: incompatible type for argument 3 of 'g_slist_foreach' 1211 | g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false); |^ || |_Bool In file included from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gmain.h:26, from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/giochannel.h:33, from /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib.h:54, from /Users/philmd/source/qemu/include/glib-compat.h:32, from /Users/philmd/source/qemu/include/qemu/osdep.h:144, from ../../block/qcow2-bitmap.c:28: /opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gslist.h:127:61: note: expected 'gpointer' {aka 'void *'} but argument is of type '_Bool' 127 | gpointer user_data); | ~~^ At top level: FAILED: libblock.fa.p/block_qcow2-bitmap.c.o Fix by adding the missing gpointer cast. Fixes: 1b6b0562db ("qcow2: support .bdrv_reopen_bitmaps_rw") Signed-off-by: Philippe Mathieu-Daudé --- Cc: Vladimir Sementsov-Ogievskiy Cc: John Snow Cc: Max Reitz --- block/qcow2-bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index ff3309846c..7197754843 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1208,7 +1208,7 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) } } -g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false); +g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, (gpointer)false); ret = 0; out: -- 2.37.3
Re: [PATCH 7/8] meson-build: Enable CONFIG_REPLICATION only when replication is set
On 02/09/2022 18.51, Juan Quintela wrote: Signed-off-by: Juan Quintela --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 20fddbd707..cab0474d0c 100644 --- a/meson.build +++ b/meson.build @@ -1878,7 +1878,7 @@ config_host_data.set('CONFIG_DEBUG_STACK_USAGE', get_option('debug_stack_usage') config_host_data.set('CONFIG_GPROF', get_option('gprof')) config_host_data.set('CONFIG_LIVE_BLOCK_MIGRATION', get_option('live_block_migration').allowed()) config_host_data.set('CONFIG_QOM_CAST_DEBUG', get_option('qom_cast_debug')) -config_host_data.set('CONFIG_REPLICATION', get_option('live_block_migration').allowed()) +config_host_data.set('CONFIG_REPLICATION', get_option('replication').allowed()) # has_header config_host_data.set('CONFIG_EPOLL', cc.has_header('sys/epoll.h')) Fixes: 406523f6b3 ("configure, meson: move block layer options to meson_options.txt") Reviewed-by: Thomas Huth
Re: [PATCH 0/8] tests: Make expliction defaults for tests
On 02/09/2022 19.16, Alexander Bulekov wrote: On 220902 1851, Juan Quintela wrote: Hi For a long, long time I have had local hacks on my tree to be able to run "make tests" when I have a minimal configure guest. This is a first try to upstream some of it. - by default we always setup -display none (it already was the default, but some places added it anyways) - by default we always setup -net none. Not clear what was the default, but no tests use the default net, so it is safe change and now it is explicit. - by default we always setup -vga none. This is a complete difference can of worms. Every tests that use vga already set vga correctly, so this is quite obvious, right? Now they are acpi tables. They are a mess. And basically this means remove a device for each one of them. Why going through all the trouble? Because while I am develping, I normall compile out vga. - Fix several error strings that were set with copy paste. - replication test requires CONFIG_REPLICATION. - test-crypto-secret requires CONFIG_SECRET_KEYRING. Please review. Except for the acpi changes (that I hope I have done right following the instructions) the rest is quite obvious. I think this might break some of the fuzz regression tests, because they have "baked-in" PCI configuration commands with hard-coded PCI addresses, which will shift around if some device is removed (e.g. with -net none). Probably the fix is to add addr=... to the -device parameter in the fuzz tests to keep the PCI address stable. -Alex The patches to default to -net none and -vga none are a good idea, but I agree with Alexander - this needs some careful examination of the fuzz tests first to see whether the BARs are changed here or not. Thomas
[PATCH v3] tcg/ppc: Optimize 26-bit jumps
PowerPC64 processors handle direct branches better than indirect ones, resulting in less stalled cycles and branch misses. However, PPC's tb_target_set_jmp_target() was only using direct branches for 16-bit jumps, while PowerPC64's unconditional branch instructions are able to handle displacements of up to 26 bits. To take advantage of this, now jumps whose displacements fit in between 17 and 26 bits are also converted to direct branches. Signed-off-by: Leandro Lupori --- v3: - make goto tb code 16-byte aligned - code cleanup v2: use stq to replace all instructions atomically tcg/ppc/tcg-target.c.inc | 105 +++ 1 file changed, 74 insertions(+), 31 deletions(-) diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc index 1cbd047ab3..0cde11c3de 100644 --- a/tcg/ppc/tcg-target.c.inc +++ b/tcg/ppc/tcg-target.c.inc @@ -1847,44 +1847,87 @@ static void tcg_out_mb(TCGContext *s, TCGArg a0) tcg_out32(s, insn); } -void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx, - uintptr_t jmp_rw, uintptr_t addr) +static inline uint64_t make_pair(tcg_insn_unit i1, tcg_insn_unit i2) { -if (TCG_TARGET_REG_BITS == 64) { -tcg_insn_unit i1, i2; -intptr_t tb_diff = addr - tc_ptr; -intptr_t br_diff = addr - (jmp_rx + 4); -uint64_t pair; - -/* This does not exercise the range of the branch, but we do - still need to be able to load the new value of TCG_REG_TB. - But this does still happen quite often. */ -if (tb_diff == (int16_t)tb_diff) { -i1 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, tb_diff); -i2 = B | (br_diff & 0x3fc); -} else { -intptr_t lo = (int16_t)tb_diff; -intptr_t hi = (int32_t)(tb_diff - lo); -assert(tb_diff == hi + lo); -i1 = ADDIS | TAI(TCG_REG_TB, TCG_REG_TB, hi >> 16); -i2 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, lo); -} -#if HOST_BIG_ENDIAN -pair = (uint64_t)i1 << 32 | i2; +if (HOST_BIG_ENDIAN) { +return (uint64_t)i1 << 32 | i2; +} +return (uint64_t)i2 << 32 | i1; +} + +static inline void ppc64_replace2(uintptr_t rx, uintptr_t rw, +tcg_insn_unit i0, tcg_insn_unit i1) +{ +#if TCG_TARGET_REG_BITS == 64 +qatomic_set((uint64_t *)rw, make_pair(i0, i1)); +flush_idcache_range(rx, rw, 8); #else -pair = (uint64_t)i2 << 32 | i1; +qemu_build_not_reached(); #endif +} -/* As per the enclosing if, this is ppc64. Avoid the _Static_assert - within qatomic_set that would fail to build a ppc32 host. */ -qatomic_set__nocheck((uint64_t *)jmp_rw, pair); -flush_idcache_range(jmp_rx, jmp_rw, 8); -} else { +static inline void ppc64_replace4(uintptr_t rx, uintptr_t rw, +tcg_insn_unit i0, tcg_insn_unit i1, tcg_insn_unit i2, tcg_insn_unit i3) +{ +uint64_t p[2]; + +p[!HOST_BIG_ENDIAN] = make_pair(i0, i1); +p[HOST_BIG_ENDIAN] = make_pair(i2, i3); + +asm("mr %%r6, %1\n\t" +"mr %%r7, %2\n\t" +"stq %%r6, %0" +: "=Q"(*(__int128 *)rw) : "r"(p[0]), "r"(p[1]) : "r6", "r7"); +flush_idcache_range(rx, rw, 16); +} + +void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx, + uintptr_t jmp_rw, uintptr_t addr) +{ +tcg_insn_unit i0, i1, i2, i3; +intptr_t tb_diff = addr - tc_ptr; +intptr_t br_diff = addr - (jmp_rx + 4); +intptr_t lo, hi; + +if (TCG_TARGET_REG_BITS == 32) { intptr_t diff = addr - jmp_rx; tcg_debug_assert(in_range_b(diff)); qatomic_set((uint32_t *)jmp_rw, B | (diff & 0x3fc)); flush_idcache_range(jmp_rx, jmp_rw, 4); +return; +} + +/* + * This does not exercise the range of the branch, but we do + * still need to be able to load the new value of TCG_REG_TB. + * But this does still happen quite often. + */ +if (tb_diff == (int16_t)tb_diff) { +i0 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, tb_diff); +i1 = B | (br_diff & 0x3fc); +ppc64_replace2(jmp_rx, jmp_rw, i0, i1); +return; +} + +lo = (int16_t)tb_diff; +hi = (int32_t)(tb_diff - lo); +assert(tb_diff == hi + lo); +i0 = ADDIS | TAI(TCG_REG_TB, TCG_REG_TB, hi >> 16); +i1 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, lo); +if (!have_isa_2_07) { +ppc64_replace2(jmp_rx, jmp_rw, i0, i1); +return; +} + +br_diff -= 4; +if (in_range_b(br_diff)) { +i2 = B | (br_diff & 0x3fc); +i3 = NOP; +} else { +i2 = MTSPR | RS(TCG_REG_TB) | CTR; +i3 = BCCTR | BO_ALWAYS; } +ppc64_replace4(jmp_rx, jmp_rw, i0, i1, i2, i3); } static void tcg_out_call_int(TCGContext *s, int lk, @@ -2574,8 +2617,8 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, if (s->tb_jmp_insn_offset) { /* Direct jump. */ if (TCG_TARGET_REG_BITS
RE: [PATCH] Hexagon (tests/tcg/hexagon): add fmin/fmax tests for signed zero
> -Original Message- > From: Matheus Tavares Bernardino > Sent: Friday, September 16, 2022 10:06 AM > To: qemu-devel@nongnu.org > Cc: Taylor Simpson ; Brian Cain > > Subject: [PATCH] Hexagon (tests/tcg/hexagon): add fmin/fmax tests for > signed zero > > Signed-off-by: Matheus Tavares Bernardino > --- > tests/tcg/hexagon/usr.c | 10 ++ > 1 file changed, 10 insertions(+) Reviewed-by: Taylor Simpson Tested-by: Taylor Simpson
[PULL 15/21] audio: add help option for -audio and -audiodev
From: Claudio Fontana add a simple help option for -audio and -audiodev to show the list of available drivers, and document them. Signed-off-by: Claudio Fontana Message-Id: <20220908081441.7111-1-cfont...@suse.de> Signed-off-by: Paolo Bonzini --- audio/audio.c | 19 +++ audio/audio.h | 1 + qemu-options.hx | 10 ++ softmmu/vl.c| 9 +++-- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 76b8735b44..cfa4119c05 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -32,6 +32,7 @@ #include "qapi/qapi-visit-audio.h" #include "qemu/cutils.h" #include "qemu/module.h" +#include "qemu/help_option.h" #include "sysemu/sysemu.h" #include "sysemu/replay.h" #include "sysemu/runstate.h" @@ -2101,10 +2102,28 @@ static void audio_validate_opts(Audiodev *dev, Error **errp) } } +void audio_help(void) +{ +int i; + +printf("Available audio drivers:\n"); + +for (i = 0; i < AUDIODEV_DRIVER__MAX; i++) { +audio_driver *driver = audio_driver_lookup(AudiodevDriver_str(i)); +if (driver) { +printf("%s\n", driver->name); +} +} +} + void audio_parse_option(const char *opt) { Audiodev *dev = NULL; +if (is_help_option(opt)) { +audio_help(); +exit(EXIT_SUCCESS); +} Visitor *v = qobject_input_visitor_new_str(opt, "driver", _fatal); visit_type_Audiodev(v, NULL, , _fatal); visit_free(v); diff --git a/audio/audio.h b/audio/audio.h index 27e67079a0..01bdc567fb 100644 --- a/audio/audio.h +++ b/audio/audio.h @@ -171,6 +171,7 @@ void audio_sample_from_uint64(void *samples, int pos, void audio_define(Audiodev *audio); void audio_parse_option(const char *opt); bool audio_init_audiodevs(void); +void audio_help(void); void audio_legacy_help(void); AudioState *audio_state_by_name(const char *name); diff --git a/qemu-options.hx b/qemu-options.hx index 1bb02363ab..d8b5ce5b43 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -704,10 +704,11 @@ SRST ``-audio [driver=]driver,model=value[,prop[=value][,...]]`` This option is a shortcut for configuring both the guest audio hardware and the host audio backend in one go. -The host backend options are the same as with the corresponding -``-audiodev`` options below. The guest hardware model can be set with -``model=modelname``. Use ``model=help`` to list the available device -types. +The driver option is the same as with the corresponding ``-audiodev`` option below. +The guest hardware model can be set with ``model=modelname``. + +Use ``driver=help`` to list the available drivers, +and ``model=help`` to list the available device types. The following two example do exactly the same, to show how ``-audio`` can be used to shorten the command line length: @@ -721,6 +722,7 @@ ERST DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev, "-audiodev [driver=]driver,id=id[,prop[=value][,...]]\n" "specifies the audio backend to use\n" +"Use ``-audiodev help`` to list the available drivers\n" "id= identifier of the backend\n" "timer-period= timer period in microseconds\n" "in|out.mixing-engine= use mixing engine to mix streams inside QEMU\n" diff --git a/softmmu/vl.c b/softmmu/vl.c index 263f029a8e..e62b9cc35d 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2842,11 +2842,16 @@ void qemu_init(int argc, char **argv, char **envp) audio_parse_option(optarg); break; case QEMU_OPTION_audio: { -QDict *dict = keyval_parse(optarg, "driver", NULL, _fatal); +bool help; char *model; Audiodev *dev = NULL; Visitor *v; - +QDict *dict = keyval_parse(optarg, "driver", , _fatal); +if (help || (qdict_haskey(dict, "driver") && + is_help_option(qdict_get_str(dict, "driver" { +audio_help(); +exit(EXIT_SUCCESS); +} if (!qdict_haskey(dict, "id")) { qdict_put_str(dict, "id", "audiodev0"); } -- 2.37.2
Re: [PATCH 8/8] meson-build: test-crypto-secret depends on CONFIG_SECRET_KEYRING
On 02/09/2022 18.51, Juan Quintela wrote: With this change "make check" works when configured with --disable-keyring. Signed-off-by: Juan Quintela --- tests/unit/meson.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/meson.build b/tests/unit/meson.build index b497a41378..988aed27cb 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -78,7 +78,6 @@ if have_block 'test-crypto-hmac': [crypto], 'test-crypto-cipher': [crypto], 'test-crypto-akcipher': [crypto], -'test-crypto-secret': [crypto, keyutils], 'test-crypto-der': [crypto], 'test-authz-simple': [authz], 'test-authz-list': [authz], @@ -122,6 +121,9 @@ if have_block if config_host_data.get('CONFIG_EPOLL_CREATE1') tests += {'test-fdmon-epoll': [testblock]} endif + if config_host_data.get('CONFIG_SECRET_KEYRING') +tests += {'test-crypto-secret': [crypto, keyutils]} + endif endif if have_system Reviewed-by: Thomas Huth
Re: [PULL 0/9] loongarch-to-apply queue
The following CI error was reported: ../hw/loongarch/virt.c: In function ‘fdt_add_irqchip_node’: ../hw/loongarch/virt.c:174:32: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=] 174 | nodename = g_strdup_printf("/intc@%" PRIx64, | ^ In file included from /builds/qemu-project/qemu/include/qemu/osdep.h:101, from ../hw/loongarch/virt.c:7: /usr/arm-linux-gnueabi/include/inttypes.h:121:34: note: format string is defined here 121 | # define PRIx64 __PRI64_PREFIX "x" https://gitlab.com/qemu-project/qemu/-/jobs/3050165217 Please fix and send a v2 pull request. Thanks! Stefan
Re: [PATCH 1/8] qtest: "-display none" is set in qtest_init()
On 02/09/2022 18.51, Juan Quintela wrote: So we don't need to set anywhere else. Signed-off-by: Juan Quintela --- tests/qtest/bios-tables-test.c | 2 +- tests/qtest/fuzz-lsi53c895a-test.c | 2 +- tests/qtest/fuzz-megasas-test.c | 2 +- tests/qtest/fuzz-sb16-test.c| 6 +++--- tests/qtest/fuzz-sdcard-test.c | 6 +++--- tests/qtest/fuzz-virtio-scsi-test.c | 2 +- tests/qtest/fuzz-xlnx-dp-test.c | 2 +- tests/qtest/fuzz/generic_fuzz.c | 3 +-- tests/qtest/fuzz/i440fx_fuzz.c | 2 +- tests/qtest/fuzz/qos_fuzz.c | 2 +- 10 files changed, 14 insertions(+), 15 deletions(-) Reviewed-by: Thomas Huth
[PULL 11/21] tests/tcg: i386: fix typos in 3DNow! instructions
Signed-off-by: Paolo Bonzini --- tests/tcg/i386/x86.csv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tcg/i386/x86.csv b/tests/tcg/i386/x86.csv index d5d0c17f1b..c43bf42dd3 100644 --- a/tests/tcg/i386/x86.csv +++ b/tests/tcg/i386/x86.csv @@ -1469,16 +1469,16 @@ "PFCMPEQ mm1, mm2/m64","PFCMPEQ mm2/m64, mm1","pfcmpeq mm2/m64, mm1","0F 0F B0 /r","V","V","3DNOW","amd","rw,r","","" "PFCMPGE mm1, mm2/m64","PFCMPGE mm2/m64, mm1","pfcmpge mm2/m64, mm1","0F 0F 90 /r","V","V","3DNOW","amd","rw,r","","" "PFCMPGT mm1, mm2/m64","PFCMPGT mm2/m64, mm1","pfcmpgt mm2/m64, mm1","0F 0F A0 /r","V","V","3DNOW","amd","rw,r","","" -"PFCPIT1 mm1, mm2/m64","PFCPIT1 mm2/m64, mm1","pfcpit1 mm2/m64, mm1","0F 0F A6 /r","V","V","3DNOW","amd","rw,r","","" "PFMAX mm1, mm2/m64","PFMAX mm2/m64, mm1","pfmax mm2/m64, mm1","0F 0F A4 /r","V","V","3DNOW","amd","rw,r","","" "PFMIN mm1, mm2/m64","PFMIN mm2/m64, mm1","pfmin mm2/m64, mm1","0F 0F 94 /r","V","V","3DNOW","amd","rw,r","","" "PFMUL mm1, mm2/m64","PFMUL mm2/m64, mm1","pfmul mm2/m64, mm1","0F 0F B4 /r","V","V","3DNOW","amd","rw,r","","" "PFNACC mm1, mm2/m64","PFNACC mm2/m64, mm1","pfnacc mm2/m64, mm1","0F 0F 8A /r","V","V","3DNOW","amd","rw,r","","" "PFPNACC mm1, mm2/m64","PFPNACC mm2/m64, mm1","pfpnacc mm2/m64, mm1","0F 0F 8E /r","V","V","3DNOW","amd","rw,r","","" "PFRCP mm1, mm2/m64","PFRCP mm2/m64, mm1","pfrcp mm2/m64, mm1","0F 0F 96 /r","V","V","3DNOW","amd","rw,r","","" +"PFRCPIT1 mm1, mm2/m64","PFRCPIT1 mm2/m64, mm1","pfrcpit1 mm2/m64, mm1","0F 0F A6 /r","V","V","3DNOW","amd","rw,r","","" "PFRCPIT2 mm1, mm2/m64","PFRCPIT2 mm2/m64, mm1","pfrcpit2 mm2/m64, mm1","0F 0F B6 /r","V","V","3DNOW","amd","rw,r","","" "PFRSQIT1 mm1, mm2/m64","PFRSQIT1 mm2/m64, mm1","pfrsqit1 mm2/m64, mm1","0F 0F A7 /r","V","V","3DNOW","amd","rw,r","","" -"PFSQRT mm1, mm2/m64","PFSQRT mm2/m64, mm1","pfsqrt mm2/m64, mm1","0F 0F 97 /r","V","V","3DNOW","amd","rw,r","","" +"PFRSQRT mm1, mm2/m64","PFRSQRT mm2/m64, mm1","pfrsqrt mm2/m64, mm1","0F 0F 97 /r","V","V","3DNOW","amd","rw,r","","" "PFSUB mm1, mm2/m64","PFSUB mm2/m64, mm1","pfsub mm2/m64, mm1","0F 0F 9A /r","V","V","3DNOW","amd","rw,r","","" "PFSUBR mm1, mm2/m64","PFSUBR mm2/m64, mm1","pfsubr mm2/m64, mm1","0F 0F AA /r","V","V","3DNOW","amd","rw,r","","" "PHADDD mm1, mm2/m64","PHADDD mm2/m64, mm1","phaddd mm2/m64, mm1","0F 38 02 /r","V","V","SSSE3","","rw,r","","" -- 2.37.2
Re: [QEMU][PATCH 4/5] tests/qtest: Introduce tests for Xilinx VERSAL CANFD controller
On 10/09/2022 08.12, Vikram Garhwal wrote: The QTests perform three tests on the Xilinx VERSAL CANFD controller: Tests the CANFD controllers in loopback. Tests the CANFD controllers in normal mode with CAN frame. Tests the CANFD controllers in normal mode with CANFD frame. Signed-off-by: Vikram Garhwal --- tests/qtest/meson.build | 1 + tests/qtest/xlnx-canfd-test.c | 421 ++ 2 files changed, 422 insertions(+) create mode 100644 tests/qtest/xlnx-canfd-test.c diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index e910cb32ca..c3802fd788 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -217,6 +217,7 @@ qtests_aarch64 = \ (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? ['tpm-tis-device-test'] : []) +\ (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? ['tpm-tis-device-swtpm-test'] : []) + \ (config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test', 'fuzz-xlnx-dp-test'] : []) + \ + (config_all_devices.has_key('CONFIG_XLNX_VERSAL') ? ['xlnx-canfd-test'] : []) + \ ['arm-cpu-features', 'numa-test', 'boot-serial-test', diff --git a/tests/qtest/xlnx-canfd-test.c b/tests/qtest/xlnx-canfd-test.c new file mode 100644 index 00..15dc03c98c --- /dev/null +++ b/tests/qtest/xlnx-canfd-test.c @@ -0,0 +1,421 @@ +/* + * QTests for the Xilinx Versal CANFD controller. + * + * Copyright (c) 2022 AMD Inc. + * + * Written-by: Vikram Garhwal + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ It's just my personal taste, but could you maybe add a SPDX license identifier in front of the license code? ... that would make it easier to identify the kind of license instead of reading through the whole text to understand which license it is. Apart from that, patch looks fine to me at a quick glance. Acked-by: Thomas Huth
Re: Call for Outreachy Dec-Mar internship project ideas
On Fri, 9 Sept 2022 at 12:41, Stefan Hajnoczi wrote: > The Outreachy open source internship program > (https://www.outreachy.org/) is running again from December-March. If > you have a project idea you'd like to mentor and are a regular > contributor to QEMU or KVM, please reply to this email by September > 22nd. Reminder: there are only a few days left for proposing Outreachy Dec-Mar project ideas. Stefan
[PULL 18/21] target/i386: REPZ and REPNZ are mutually exclusive
The later prefix wins if both are present, make it show in s->prefix too. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/tcg/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 5f31a59fb8..eaa56b0f48 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -4733,9 +4733,11 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) switch (b) { case 0xf3: prefixes |= PREFIX_REPZ; +prefixes &= ~PREFIX_REPNZ; goto next_byte; case 0xf2: prefixes |= PREFIX_REPNZ; +prefixes &= ~PREFIX_REPZ; goto next_byte; case 0xf0: prefixes |= PREFIX_LOCK; -- 2.37.2
Re: Travis CI webhook returns HTTP 500
On 19/09/2022 19.04, Stefan Hajnoczi wrote: GitLab sends qemu.git push event webhooks to Travis CI. Recently the webhooks have been failing with HTTP 500 Internal Server Error. Do you know how to resolve this or who configured Travis CI webhooks for QEMU? I haven't been involved in this, but IIRC Paolo set up the Travis CI for QEMU? Thomas
[PULL 10/21] tests: unit: add NULL-pointer check
In CID 1432593, Coverity complains that the result of qdict_crumple() might leak if it is not a dictionary. This is not a practical concern since the test would fail immediately with a NULL pointer dereference in qdict_size(). However, it is not nice to depend on qdict_size() crashing, so add an explicit assertion that that the crumpled object was indeed a dictionary. Signed-off-by: Paolo Bonzini --- tests/unit/check-block-qdict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/check-block-qdict.c b/tests/unit/check-block-qdict.c index 5a25825093..751c58e737 100644 --- a/tests/unit/check-block-qdict.c +++ b/tests/unit/check-block-qdict.c @@ -504,7 +504,7 @@ static void qdict_crumple_test_empty(void) src = qdict_new(); dst = qobject_to(QDict, qdict_crumple(src, _abort)); - +g_assert(dst); g_assert_cmpint(qdict_size(dst), ==, 0); qobject_unref(src); -- 2.37.2
[PULL 03/21] kvm: fix memory leak on failure to read stats descriptors
Reported by Coverity as CID 1490142. Since the size is constant and the lifetime is the same as the StatsDescriptors struct, embed the struct directly instead of using a separate allocation. Suggested-by: Richard Henderson Signed-off-by: Paolo Bonzini --- accel/kvm/kvm-all.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 7c8ce18bdd..5acab1767f 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -3908,7 +3908,7 @@ exit: typedef struct StatsDescriptors { const char *ident; /* cache key, currently the StatsTarget */ struct kvm_stats_desc *kvm_stats_desc; -struct kvm_stats_header *kvm_stats_header; +struct kvm_stats_header kvm_stats_header; QTAILQ_ENTRY(StatsDescriptors) next; } StatsDescriptors; @@ -3939,7 +3939,7 @@ static StatsDescriptors *find_stats_descriptors(StatsTarget target, int stats_fd descriptors = g_new0(StatsDescriptors, 1); /* Read stats header */ -kvm_stats_header = g_malloc(sizeof(*kvm_stats_header)); +kvm_stats_header = >kvm_stats_header; ret = read(stats_fd, kvm_stats_header, sizeof(*kvm_stats_header)); if (ret != sizeof(*kvm_stats_header)) { error_setg(errp, "KVM stats: failed to read stats header: " @@ -3964,7 +3964,6 @@ static StatsDescriptors *find_stats_descriptors(StatsTarget target, int stats_fd g_free(kvm_stats_desc); return NULL; } -descriptors->kvm_stats_header = kvm_stats_header; descriptors->kvm_stats_desc = kvm_stats_desc; descriptors->ident = ident; QTAILQ_INSERT_TAIL(_descriptors, descriptors, next); @@ -3989,7 +3988,7 @@ static void query_stats(StatsResultList **result, StatsTarget target, return; } -kvm_stats_header = descriptors->kvm_stats_header; +kvm_stats_header = >kvm_stats_header; kvm_stats_desc = descriptors->kvm_stats_desc; size_desc = sizeof(*kvm_stats_desc) + kvm_stats_header->name_size; @@ -4054,7 +4053,7 @@ static void query_stats_schema(StatsSchemaList **result, StatsTarget target, return; } -kvm_stats_header = descriptors->kvm_stats_header; +kvm_stats_header = >kvm_stats_header; kvm_stats_desc = descriptors->kvm_stats_desc; size_desc = sizeof(*kvm_stats_desc) + kvm_stats_header->name_size; -- 2.37.2
[PULL 20/21] build: remove extra parentheses causing missing rebuilds
Because of two stray parentheses at the end of the definition of ninja-cmd-goals, the test that is last in the .check-TESTSUITENAME.deps variable will not be rebuilt. Fix that. Signed-off-by: Paolo Bonzini --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 13234f2aa4..b576cba5a8 100644 --- a/Makefile +++ b/Makefile @@ -145,7 +145,7 @@ NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \ $(filter-out -j, $(lastword -j1 $(filter -l% -j%, $(MAKEFLAGS \ -d keepdepfile ninja-cmd-goals = $(or $(MAKECMDGOALS), all) -ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g +ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g)) makefile-targets := build.ninja ctags TAGS cscope dist clean uninstall # "ninja -t targets" also lists all prerequisites. If build system -- 2.37.2
[PULL 19/21] target/i386: introduce insn_get_addr
The "O" operand type in the Intel SDM needs to load an 8- to 64-bit unsigned value, while insn_get is limited to 32 bits. Extract the code out of disas_insn and into a separate function. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/tcg/translate.c | 36 ++-- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index eaa56b0f48..44af8c107f 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -2289,6 +2289,31 @@ static void gen_ldst_modrm(CPUX86State *env, DisasContext *s, int modrm, } } +static target_ulong insn_get_addr(CPUX86State *env, DisasContext *s, MemOp ot) +{ +target_ulong ret; + +switch (ot) { +case MO_8: +ret = x86_ldub_code(env, s); +break; +case MO_16: +ret = x86_lduw_code(env, s); +break; +case MO_32: +ret = x86_ldl_code(env, s); +break; +#ifdef TARGET_X86_64 +case MO_64: +ret = x86_ldq_code(env, s); +break; +#endif +default: +g_assert_not_reached(); +} +return ret; +} + static inline uint32_t insn_get(CPUX86State *env, DisasContext *s, MemOp ot) { uint32_t ret; @@ -5851,16 +5876,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) target_ulong offset_addr; ot = mo_b_d(b, dflag); -switch (s->aflag) { -#ifdef TARGET_X86_64 -case MO_64: -offset_addr = x86_ldq_code(env, s); -break; -#endif -default: -offset_addr = insn_get(env, s, s->aflag); -break; -} +offset_addr = insn_get_addr(env, s, s->aflag); tcg_gen_movi_tl(s->A0, offset_addr); gen_add_A0_ds_seg(s); if ((b & 2) == 0) { -- 2.37.2
[PULL 21/21] qboot: update to latest submodule
Include patch "Place setup_data at location specified by host" from Jason A. Donenfeld. Cc: Jason A. Donenfeld Signed-off-by: Paolo Bonzini --- roms/qboot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roms/qboot b/roms/qboot index a5300c4949..8ca302e86d 16 --- a/roms/qboot +++ b/roms/qboot @@ -1 +1 @@ -Subproject commit a5300c4949b8d4de2d34bedfaed66793f48ec948 +Subproject commit 8ca302e86d685fa05b16e2b20243da319941 -- 2.37.2
[PULL 17/21] target/i386: fix INSERTQ implementation
INSERTQ is defined to not modify any bits in the lower 64 bits of the destination, other than the ones being replaced with bits from the source operand. QEMU instead is using unshifted bits from the source for those bits. Signed-off-by: Paolo Bonzini --- target/i386/ops_sse.h| 10 +- target/i386/ops_sse_header.h | 2 +- target/i386/tcg/translate.c | 14 -- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h index 3504bca36a..7bf8bb967d 100644 --- a/target/i386/ops_sse.h +++ b/target/i386/ops_sse.h @@ -934,7 +934,7 @@ void helper_extrq_i(CPUX86State *env, ZMMReg *d, int index, int length) d->ZMM_Q(0) = helper_extrq(d->ZMM_Q(0), index, length); } -static inline uint64_t helper_insertq(uint64_t src, int shift, int len) +static inline uint64_t helper_insertq(uint64_t dest, uint64_t src, int shift, int len) { uint64_t mask; @@ -943,17 +943,17 @@ static inline uint64_t helper_insertq(uint64_t src, int shift, int len) } else { mask = (1ULL << len) - 1; } -return (src & ~(mask << shift)) | ((src & mask) << shift); +return (dest & ~(mask << shift)) | ((src & mask) << shift); } void helper_insertq_r(CPUX86State *env, ZMMReg *d, ZMMReg *s) { -d->ZMM_Q(0) = helper_insertq(s->ZMM_Q(0), s->ZMM_B(9) & 63, s->ZMM_B(8) & 63); +d->ZMM_Q(0) = helper_insertq(d->ZMM_Q(0), s->ZMM_Q(0), s->ZMM_B(9) & 63, s->ZMM_B(8) & 63); } -void helper_insertq_i(CPUX86State *env, ZMMReg *d, int index, int length) +void helper_insertq_i(CPUX86State *env, ZMMReg *d, ZMMReg *s, int index, int length) { -d->ZMM_Q(0) = helper_insertq(d->ZMM_Q(0), index, length); +d->ZMM_Q(0) = helper_insertq(d->ZMM_Q(0), s->ZMM_Q(0), index, length); } #endif diff --git a/target/i386/ops_sse_header.h b/target/i386/ops_sse_header.h index d99464afb0..400b24c091 100644 --- a/target/i386/ops_sse_header.h +++ b/target/i386/ops_sse_header.h @@ -193,7 +193,7 @@ DEF_HELPER_3(rcpss, void, env, ZMMReg, ZMMReg) DEF_HELPER_3(extrq_r, void, env, ZMMReg, ZMMReg) DEF_HELPER_4(extrq_i, void, env, ZMMReg, int, int) DEF_HELPER_3(insertq_r, void, env, ZMMReg, ZMMReg) -DEF_HELPER_4(insertq_i, void, env, ZMMReg, int, int) +DEF_HELPER_5(insertq_i, void, env, ZMMReg, ZMMReg, int, int) DEF_HELPER_3(glue(haddps, SUFFIX), void, env, ZMMReg, ZMMReg) DEF_HELPER_3(glue(haddpd, SUFFIX), void, env, ZMMReg, ZMMReg) DEF_HELPER_3(glue(hsubps, SUFFIX), void, env, ZMMReg, ZMMReg) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 8ec91d17af..5f31a59fb8 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -3506,10 +3506,20 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, gen_helper_extrq_i(cpu_env, s->ptr0, tcg_const_i32(bit_index), tcg_const_i32(field_length)); -else -gen_helper_insertq_i(cpu_env, s->ptr0, +else { +if (mod != 3) { +gen_lea_modrm(env, s, modrm); +op2_offset = offsetof(CPUX86State, xmm_t0); +gen_ldq_env_A0(s, offsetof(CPUX86State, xmm_t0.ZMM_D(0))); +} else { +rm = (modrm & 7) | REX_B(s); +op2_offset = ZMM_OFFSET(rm); +} +tcg_gen_addi_ptr(s->ptr1, cpu_env, op2_offset); +gen_helper_insertq_i(cpu_env, s->ptr0, s->ptr1, tcg_const_i32(bit_index), tcg_const_i32(field_length)); +} } break; case 0x7e: /* movd ea, mm */ -- 2.37.2
[PULL 14/21] tests/tcg: remove old SSE tests
The new testsuite is much more comprehensive, so remove the old one; it is also buggy (the pinsrw test uses incorrect constraints, with = instead of +, and the golden output for the fxsave tests differs depending on how the C library uses SSE and AVX instructions). Signed-off-by: Paolo Bonzini --- tests/tcg/i386/test-i386.c | 573 - 1 file changed, 573 deletions(-) diff --git a/tests/tcg/i386/test-i386.c b/tests/tcg/i386/test-i386.c index e6b308a2c0..864c4e620d 100644 --- a/tests/tcg/i386/test-i386.c +++ b/tests/tcg/i386/test-i386.c @@ -34,15 +34,8 @@ #endif //#define LINUX_VM86_IOPL_FIX //#define TEST_P4_FLAGS -#ifdef __SSE__ -#define TEST_SSE #define TEST_CMOV 1 #define TEST_FCOMI 1 -#else -#undef TEST_SSE -#define TEST_CMOV 1 -#define TEST_FCOMI 1 -#endif #if defined(__x86_64__) #define FMT64X "%016lx" @@ -2104,568 +2097,6 @@ static void test_enter(void) TEST_ENTER("w", uint16_t, 31); } -#ifdef TEST_SSE - -typedef int __m64 __attribute__ ((vector_size(8))); -typedef float __m128 __attribute__ ((vector_size(16))); - -typedef union { -double d[2]; -float s[4]; -uint32_t l[4]; -uint64_t q[2]; -__m128 dq; -} XMMReg; - -static uint64_t __attribute__((aligned(16))) test_values[4][2] = { -{ 0x456723c698694873, 0xdc515cff944a58ec }, -{ 0x1f297ccd58bad7ab, 0x41f21efba9e3e146 }, -{ 0x007c62c2085427f8, 0x231be9e8cde7438d }, -{ 0x0f76255a085427f8, 0xc233e9e8c4c9439a }, -}; - -#define SSE_OP(op)\ -{\ -asm volatile (#op " %2, %0" : "=x" (r.dq) : "0" (a.dq), "x" (b.dq));\ -printf("%-9s: a=" FMT64X "" FMT64X " b=" FMT64X "" FMT64X " r=" FMT64X "" FMT64X "\n",\ - #op,\ - a.q[1], a.q[0],\ - b.q[1], b.q[0],\ - r.q[1], r.q[0]);\ -} - -#define SSE_OP2(op)\ -{\ -int i;\ -for(i=0;i<2;i++) {\ -a.q[0] = test_values[2*i][0];\ -a.q[1] = test_values[2*i][1];\ -b.q[0] = test_values[2*i+1][0];\ -b.q[1] = test_values[2*i+1][1];\ -SSE_OP(op);\ -}\ -} - -#define MMX_OP2(op)\ -{\ -int i;\ -for(i=0;i<2;i++) {\ -a.q[0] = test_values[2*i][0];\ -b.q[0] = test_values[2*i+1][0];\ -asm volatile (#op " %2, %0" : "=y" (r.q[0]) : "0" (a.q[0]), "y" (b.q[0]));\ -printf("%-9s: a=" FMT64X " b=" FMT64X " r=" FMT64X "\n",\ - #op,\ - a.q[0],\ - b.q[0],\ - r.q[0]);\ -}\ -SSE_OP2(op);\ -} - -#define SHUF_OP(op, ib)\ -{\ -a.q[0] = test_values[0][0];\ -a.q[1] = test_values[0][1];\ -b.q[0] = test_values[1][0];\ -b.q[1] = test_values[1][1];\ -asm volatile (#op " $" #ib ", %2, %0" : "=x" (r.dq) : "0" (a.dq), "x" (b.dq));\ -printf("%-9s: a=" FMT64X "" FMT64X " b=" FMT64X "" FMT64X " ib=%02x r=" FMT64X "" FMT64X "\n",\ - #op,\ - a.q[1], a.q[0],\ - b.q[1], b.q[0],\ - ib,\ - r.q[1], r.q[0]);\ -} - -#define PSHUF_OP(op, ib)\ -{\ -int i;\ -for(i=0;i<2;i++) {\ -a.q[0] = test_values[2*i][0];\ -a.q[1] = test_values[2*i][1];\ -asm volatile (#op " $" #ib ", %1, %0" : "=x" (r.dq) : "x" (a.dq));\ -printf("%-9s: a=" FMT64X "" FMT64X " ib=%02x r=" FMT64X "" FMT64X "\n",\ - #op,\ - a.q[1], a.q[0],\ - ib,\ - r.q[1], r.q[0]);\ -}\ -} - -#define SHIFT_IM(op, ib)\ -{\ -int i;\ -for(i=0;i<2;i++) {\ -a.q[0] = test_values[2*i][0];\ -a.q[1] = test_values[2*i][1];\ -asm volatile (#op " $" #ib ", %0" : "=x" (r.dq) : "0" (a.dq));\ -printf("%-9s: a=" FMT64X "" FMT64X " ib=%02x r=" FMT64X "" FMT64X "\n",\ - #op,\ - a.q[1], a.q[0],\ - ib,\ - r.q[1], r.q[0]);\ -}\ -} - -#define SHIFT_OP(op, ib)\ -{\ -int i;\ -SHIFT_IM(op, ib);\ -for(i=0;i<2;i++) {\ -a.q[0] = test_values[2*i][0];\ -a.q[1] = test_values[2*i][1];\ -b.q[0] = ib;\ -b.q[1] = 0;\ -asm volatile (#op " %2, %0" : "=x" (r.dq) : "0" (a.dq), "x" (b.dq));\ -printf("%-9s: a=" FMT64X "" FMT64X " b=" FMT64X "" FMT64X " r=" FMT64X "" FMT64X "\n",\ - #op,\ - a.q[1], a.q[0],\ - b.q[1], b.q[0],\ - r.q[1], r.q[0]);\ -}\ -} - -#define MOVMSK(op)\ -{\ -int i, reg;\ -for(i=0;i<2;i++) {\ -a.q[0] = test_values[2*i][0];\ -a.q[1] = test_values[2*i][1];\ -asm volatile (#op " %1, %0" : "=r" (reg) : "x" (a.dq));\ -printf("%-9s: a=" FMT64X "" FMT64X " r=%08x\n",\ - #op,\ - a.q[1], a.q[0],\ - reg);\ -}\ -} - -#define SSE_OPS(a) \ -SSE_OP(a ## ps);\ -SSE_OP(a ## ss); - -#define SSE_OPD(a) \ -SSE_OP(a ## pd);\ -SSE_OP(a ## sd); - -#define SSE_COMI(op, field)\ -{\ -unsigned long eflags;\ -XMMReg a, b;\ -a.field[0] = a1;\ -b.field[0] = b1;\ -asm volatile (#op " %2, %1\n"\ -"pushf\n"\ -"pop %0\n"\ -: "=rm" (eflags)\ -: "x" (a.dq), "x" (b.dq));\ -printf("%-9s: a=%f b=%f cc=%04lx\n",\ - #op, a1, b1,\ -
[PULL 04/21] spapr_pci: fix leak in spapr_phb_vfio_get_loc_code
Overwriting "path" in the second call to g_strdup_printf() causes a memory leak, even if the variable itself is g_autofree. Reported by Coverity as CID 1460454. Signed-off-by: Paolo Bonzini --- hw/ppc/spapr_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 67e9d468aa..57c8a4f085 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -800,6 +800,7 @@ static char *spapr_phb_vfio_get_loc_code(SpaprPhbState *sphb, PCIDevice *pdev) } /* Construct and read from host device tree the loc-code */ +g_free(path); path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", devspec); if (!g_file_get_contents(path, , NULL, NULL)) { return NULL; -- 2.37.2
[PULL 16/21] target/i386: correctly mask SSE4a bit indices in register operands
SSE4a instructions EXTRQ and INSERTQ have two bit index operands, that can be immediates or taken from an XMM register. In both cases, the fields are 6-bit wide and the top two bits in the byte are ignored. translate.c is doing that correctly for the immediate case, but not for the XMM case, so fix it. Signed-off-by: Paolo Bonzini --- target/i386/ops_sse.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h index c0766de18d..3504bca36a 100644 --- a/target/i386/ops_sse.h +++ b/target/i386/ops_sse.h @@ -926,7 +926,7 @@ static inline uint64_t helper_extrq(uint64_t src, int shift, int len) void helper_extrq_r(CPUX86State *env, ZMMReg *d, ZMMReg *s) { -d->ZMM_Q(0) = helper_extrq(d->ZMM_Q(0), s->ZMM_B(1), s->ZMM_B(0)); +d->ZMM_Q(0) = helper_extrq(d->ZMM_Q(0), s->ZMM_B(1) & 63, s->ZMM_B(0) & 63); } void helper_extrq_i(CPUX86State *env, ZMMReg *d, int index, int length) @@ -948,7 +948,7 @@ static inline uint64_t helper_insertq(uint64_t src, int shift, int len) void helper_insertq_r(CPUX86State *env, ZMMReg *d, ZMMReg *s) { -d->ZMM_Q(0) = helper_insertq(s->ZMM_Q(0), s->ZMM_B(9), s->ZMM_B(8)); +d->ZMM_Q(0) = helper_insertq(s->ZMM_Q(0), s->ZMM_B(9) & 63, s->ZMM_B(8) & 63); } void helper_insertq_i(CPUX86State *env, ZMMReg *d, int index, int length) -- 2.37.2
[PULL 12/21] tests/tcg: i386: add MMX and 3DNow! tests
Adjust the test-avx.py generator to produce tests specifically for MMX and 3DNow. Using a separate generator introduces some code duplication, but is a simpler approach because of test-avx's extra complexity to support 3- and 4-operand AVX instructions. If needed, a common library can be introduced later. While at it, for consistency move all the -cpu max rules to the same place. Signed-off-by: Paolo Bonzini --- tests/tcg/i386/Makefile.target | 24 ++- tests/tcg/i386/test-3dnow.c | 3 + tests/tcg/i386/test-avx.py | 1 - tests/tcg/i386/test-mmx.c| 315 +++ tests/tcg/i386/test-mmx.py | 244 tests/tcg/x86_64/Makefile.target | 1 - 6 files changed, 583 insertions(+), 5 deletions(-) create mode 100644 tests/tcg/i386/test-3dnow.c create mode 100644 tests/tcg/i386/test-mmx.c create mode 100755 tests/tcg/i386/test-mmx.py diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target index be21b81b96..599f192529 100644 --- a/tests/tcg/i386/Makefile.target +++ b/tests/tcg/i386/Makefile.target @@ -7,8 +7,8 @@ VPATH += $(I386_SRC) I386_SRCS=$(notdir $(wildcard $(I386_SRC)/*.c)) ALL_X86_TESTS=$(I386_SRCS:.c=) -SKIP_I386_TESTS=test-i386-ssse3 test-avx -X86_64_TESTS:=$(filter test-i386-bmi2 test-i386-ssse3 test-avx, $(ALL_X86_TESTS)) +SKIP_I386_TESTS=test-i386-ssse3 test-avx test-3dnow test-mmx +X86_64_TESTS:=$(filter test-i386-bmi2 $(SKIP_I386_TESTS), $(ALL_X86_TESTS)) test-i386-sse-exceptions: CFLAGS += -msse4.1 -mfpmath=sse run-test-i386-sse-exceptions: QEMU_OPTS += -cpu max @@ -82,9 +82,27 @@ run-plugin-sha512-sse-with-%: QEMU_OPTS+=-cpu max TESTS+=sha512-sse -CLEANFILES += test-avx.h +CLEANFILES += test-avx.h test-mmx.h test-3dnow.h +test-3dnow.h: test-mmx.py x86.csv + $(PYTHON) $(I386_SRC)/test-mmx.py $(I386_SRC)/x86.csv $@ 3DNOW + +test-mmx.h: test-mmx.py x86.csv + $(PYTHON) $(I386_SRC)/test-mmx.py $(I386_SRC)/x86.csv $@ MMX SSE SSE2 SSE3 SSSE3 + test-avx.h: test-avx.py x86.csv $(PYTHON) $(I386_SRC)/test-avx.py $(I386_SRC)/x86.csv $@ +test-3dnow: CFLAGS += -masm=intel -O -I. +run-test-3dnow: QEMU_OPTS += -cpu max +run-plugin-test-3dnow: QEMU_OPTS += -cpu max +test-3dnow: test-3dnow.h + +test-mmx: CFLAGS += -masm=intel -O -I. +run-test-mmx: QEMU_OPTS += -cpu max +run-plugin-test-mmx: QEMU_OPTS += -cpu max +test-mmx: test-mmx.h + test-avx: CFLAGS += -masm=intel -O -I. +run-test-avx: QEMU_OPTS += -cpu max +run-plugin-test-avx: QEMU_OPTS += -cpu max test-avx: test-avx.h diff --git a/tests/tcg/i386/test-3dnow.c b/tests/tcg/i386/test-3dnow.c new file mode 100644 index 00..67abc68677 --- /dev/null +++ b/tests/tcg/i386/test-3dnow.c @@ -0,0 +1,3 @@ +#define EMMS "femms" +#define TEST_FILE "test-3dnow.h" +#include "test-mmx.c" diff --git a/tests/tcg/i386/test-avx.py b/tests/tcg/i386/test-avx.py index 6eb455a8b4..2516c66445 100755 --- a/tests/tcg/i386/test-avx.py +++ b/tests/tcg/i386/test-avx.py @@ -7,7 +7,6 @@ from fnmatch import fnmatch archs = [ -# TODO: MMX? "SSE", "SSE2", "SSE3", "SSSE3", "SSE4_1", "SSE4_2", ] diff --git a/tests/tcg/i386/test-mmx.c b/tests/tcg/i386/test-mmx.c new file mode 100644 index 00..60802067d4 --- /dev/null +++ b/tests/tcg/i386/test-mmx.c @@ -0,0 +1,315 @@ +#include +#include +#include +#include + +#ifndef TEST_FILE +#define TEST_FILE "test-mmx.h" +#endif +#ifndef EMMS +#define EMMS "emms" +#endif + +typedef void (*testfn)(void); + +typedef struct { +uint64_t q0, q1; +} __attribute__((aligned(16))) v2di; + +typedef struct { +uint64_t mm[8]; +v2di xmm[8]; +uint64_t r[16]; +uint64_t flags; +uint32_t ff; +uint64_t pad; +v2di mem[4]; +v2di mem0[4]; +} reg_state; + +typedef struct { +int n; +testfn fn; +const char *s; +reg_state *init; +} TestDef; + +reg_state initI; +reg_state initF32; +reg_state initF64; + +static void dump_mmx(int n, const uint64_t *r, int ff) +{ +if (ff == 32) { +float v[2]; +memcpy(v, r, sizeof(v)); +printf("MM%d = %016lx %8g %8g\n", n, *r, v[1], v[0]); +} else { +printf("MM%d = %016lx\n", n, *r); +} +} + +static void dump_xmm(const char *name, int n, const v2di *r, int ff) +{ +printf("%s%d = %016lx %016lx\n", + name, n, r->q1, r->q0); +if (ff == 32) { +float v[4]; +memcpy(v, r, sizeof(v)); +printf(" %8g %8g %8g %8g\n", +v[3], v[2], v[1], v[0]); +} +} + +static void dump_regs(reg_state *s, int ff) +{ +int i; + +for (i = 0; i < 8; i++) { +dump_mmx(i, >mm[i], ff); +} +for (i = 0; i < 4; i++) { +dump_xmm("mem", i, >mem0[i], 0); +} +} + +static void compare_state(const reg_state *a, const reg_state *b) +{ +int i; +for (i = 0; i < 8; i++) { +if (a->mm[i] != b->mm[i]) { +printf("MM%d = %016lx\n", i, b->mm[i]); +} +} +for (i = 0; i < 16; i++) { +if
[PULL 01/21] KVM: use store-release to mark dirty pages as harvested
The following scenario can happen if QEMU sets more RESET flags while the KVM_RESET_DIRTY_RINGS ioctl is ongoing on another host CPU: CPU0 CPU1 CPU2 -- fill gfn0 store-rel flags for gfn0 fill gfn1 store-rel flags for gfn1 load-acq flags for gfn0 set RESET for gfn0 load-acq flags for gfn1 set RESET for gfn1 do ioctl! ---> ioctl(RESET_RINGS) fill gfn2 store-rel flags for gfn2 load-acq flags for gfn2 set RESET for gfn2 process gfn0 process gfn1 process gfn2 do ioctl! etc. The three load-acquire in CPU0 synchronize with the three store-release in CPU2, but CPU0 and CPU1 are only synchronized up to gfn1 and CPU1 may miss gfn2's fields other than flags. The kernel must be able to cope with invalid values of the fields, and userspace *will* invoke the ioctl once more. However, once the RESET flag is cleared on gfn2, it is lost forever, therefore in the above scenario CPU1 must read the correct value of gfn2's fields. Therefore RESET must be set with a store-release, that will synchronize with KVM's load-acquire in CPU1. Cc: Gavin Shan Reviewed-by: Peter Xu Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- accel/kvm/kvm-all.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 136c8eaed3..7c8ce18bdd 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -728,7 +728,23 @@ static bool dirty_gfn_is_dirtied(struct kvm_dirty_gfn *gfn) static void dirty_gfn_set_collected(struct kvm_dirty_gfn *gfn) { -gfn->flags = KVM_DIRTY_GFN_F_RESET; +/* + * Use a store-release so that the CPU that executes KVM_RESET_DIRTY_RINGS + * sees the full content of the ring: + * + * CPU0 CPU1 CPU2 + * -- + * fill gfn0 + * store-rel flags for gfn0 + * load-acq flags for gfn0 + * store-rel RESET for gfn0 + * ioctl(RESET_RINGS) + *load-acq flags for gfn0 + *check if flags have RESET + * + * The synchronization goes from CPU2 to CPU0 to CPU1. + */ +qatomic_store_release(>flags, KVM_DIRTY_GFN_F_RESET); } /* -- 2.37.2
[PULL 13/21] tests/tcg: refine MMX support in SSE tests
Extend the support to memory operands, and skip MMX instructions that were introduced in SSE times, because they are now covered in test-mmx. Signed-off-by: Paolo Bonzini --- tests/tcg/i386/test-avx.py | 32 ++-- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/tcg/i386/test-avx.py b/tests/tcg/i386/test-avx.py index 2516c66445..e16a3d8bee 100755 --- a/tests/tcg/i386/test-avx.py +++ b/tests/tcg/i386/test-avx.py @@ -103,7 +103,11 @@ def regstr(self, n): class MMArg(): isxmm = True -ismem = False # TODO +def __init__(self, mw): +if mw not in [0, 32, 64]: +raise Exception("Bad mem width: %s" % mw) +self.mw = mw +self.ismem = mw != 0 def regstr(self, n): return "mm%d" % (n & 7) @@ -169,6 +173,9 @@ def __init__(self, w): def regstr(self, n): return mem_w(self.w) +class SkipInstruction(Exception): +pass + def ArgGenerator(arg, op): if arg[:3] == 'xmm' or arg[:3] == "ymm": if "/" in arg: @@ -179,7 +186,13 @@ def ArgGenerator(arg, op): else: return XMMArg(arg[0], 0); elif arg[:2] == 'mm': -return MMArg(); +if "/" in arg: +r, m = arg.split('/') +if (m[0] != 'm'): +raise Exception("Expected /m: %s", arg) +return MMArg(int(m[1:])); +else: +return MMArg(0); elif arg[:4] == 'imm8': return ArgImm8u(op); elif arg == '': @@ -217,8 +230,12 @@ def __init__(self, op, args): try: self.args = list(ArgGenerator(a, op) for a in args) +if not any((x.isxmm for x in self.args)): +raise SkipInstruction if len(self.args) > 0 and self.args[-1] is None: self.args = self.args[:-1] +except SkipInstruction: +raise except Exception as e: raise Exception("Bad arg %s: %s" % (op, e)) @@ -339,10 +356,13 @@ def main(): continue cpuid = row[6] if cpuid in archs: -g = InsnGenerator(insn[0], insn[1:]) -for insn in g.gen(): -outf.write('TEST(%d, "%s", %s)\n' % (n, insn, g.optype)) -n += 1 +try: +g = InsnGenerator(insn[0], insn[1:]) +for insn in g.gen(): +outf.write('TEST(%d, "%s", %s)\n' % (n, insn, g.optype)) +n += 1 +except SkipInstruction: +pass outf.write("#undef TEST\n") csvfile.close() -- 2.37.2
[PULL 09/21] tests: test-qga: close socket on failure to connect
Reported by Coverity as CID 1432543. Signed-off-by: Paolo Bonzini --- tests/unit/test-qga.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c index a05a4628ed..d27ff94d13 100644 --- a/tests/unit/test-qga.c +++ b/tests/unit/test-qga.c @@ -32,6 +32,7 @@ static int connect_qga(char *path) g_usleep(G_USEC_PER_SEC); } if (i++ == 10) { +close(s); return -1; } } while (ret == -1); -- 2.37.2
[PULL 05/21] coverity: add new RISC-V component
Signed-off-by: Paolo Bonzini --- scripts/coverity-scan/COMPONENTS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 3aad9cdfaf..fc1608932e 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -146,3 +146,6 @@ tests loongarch ~ (/qemu)?((/include)?/hw/(loongarch/.*|.*/loongarch.*)|/target/loongarch/.*) + +riscv + ~ (/qemu)?((/include)?/hw/riscv/.*|/target/riscv/.*|/hw/.*/(riscv_|ibex_|sifive_).*) -- 2.37.2
Re: [PATCH v3 2/2] target/i386: Raise #GP on unaligned m128 accesses when required.
Hi, I will merge this patch already, but with simpler code that doesn't look at PREFIX_VEX. The changes to the helpers and the addition of the aligned argument help with the new decoder as well, and I will build on top of them in the next submission of the AVX patches. Thanks! Paolo On Sat, Sep 17, 2022 at 4:15 AM Ricky Zhou wrote: > > Checking back on the status of patch, I noticed that there are some > exciting patches out for AVX support that may conflict with this, > though I see that they are still in the RFC phase: > https://patchew.org/QEMU/20220911230418.340941-1-pbonz...@redhat.com/ > > I'm not sure how far away AVX support is from being merged, but do let > me know if there's any preference re applying this change vs. waiting > to rebase on top the AVX support changes, etc. > > Thanks! > Ricky > > On Mon, Aug 29, 2022 at 8:48 PM Ricky Zhou wrote: > > > > Many instructions which load/store 128-bit values are supposed to > > raise #GP when the memory operand isn't 16-byte aligned. This includes: > > - Instructions explicitly requiring memory alignment (Exceptions Type 1 > >in the "AVX and SSE Instruction Exception Specification" section of > >the SDM) > > - Legacy SSE instructions that load/store 128-bit values (Exceptions > >Types 2 and 4). > > > > This change sets MO_ALIGN_16 on 128-bit memory accesses that require > > 16-byte alignment. It adds cpu_record_sigbus and cpu_do_unaligned_access > > hooks that simulate a #GP exception in qemu-user and qemu-system, > > respectively. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/217 > > Reviewed-by: Richard Henderson > > Signed-off-by: Ricky Zhou > > --- > > target/i386/tcg/excp_helper.c| 13 > > target/i386/tcg/helper-tcg.h | 28 ++--- > > target/i386/tcg/sysemu/excp_helper.c | 8 + > > target/i386/tcg/tcg-cpu.c| 2 ++ > > target/i386/tcg/translate.c | 45 +--- > > target/i386/tcg/user/excp_helper.c | 7 + > > 6 files changed, 74 insertions(+), 29 deletions(-) > > > > diff --git a/target/i386/tcg/excp_helper.c b/target/i386/tcg/excp_helper.c > > index c1ffa1c0ef..7c3c8dc7fe 100644 > > --- a/target/i386/tcg/excp_helper.c > > +++ b/target/i386/tcg/excp_helper.c > > @@ -140,3 +140,16 @@ G_NORETURN void raise_exception_ra(CPUX86State *env, > > int exception_index, > > { > > raise_interrupt2(env, exception_index, 0, 0, 0, retaddr); > > } > > + > > +G_NORETURN void handle_unaligned_access(CPUX86State *env, vaddr vaddr, > > +MMUAccessType access_type, > > +uintptr_t retaddr) > > +{ > > +/* > > + * Unaligned accesses are currently only triggered by SSE/AVX > > + * instructions that impose alignment requirements on memory > > + * operands. These instructions raise #GP(0) upon accessing an > > + * unaligned address. > > + */ > > +raise_exception_ra(env, EXCP0D_GPF, retaddr); > > +} > > diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h > > index 34167e2e29..cd1723389a 100644 > > --- a/target/i386/tcg/helper-tcg.h > > +++ b/target/i386/tcg/helper-tcg.h > > @@ -42,17 +42,6 @@ void x86_cpu_do_interrupt(CPUState *cpu); > > bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req); > > #endif > > > > -/* helper.c */ > > -#ifdef CONFIG_USER_ONLY > > -void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr, > > -MMUAccessType access_type, > > -bool maperr, uintptr_t ra); > > -#else > > -bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > > - MMUAccessType access_type, int mmu_idx, > > - bool probe, uintptr_t retaddr); > > -#endif > > - > > void breakpoint_handler(CPUState *cs); > > > > /* n must be a constant to be efficient */ > > @@ -78,6 +67,23 @@ G_NORETURN void raise_exception_err_ra(CPUX86State *env, > > int exception_index, > > int error_code, uintptr_t retaddr); > > G_NORETURN void raise_interrupt(CPUX86State *nenv, int intno, int is_int, > > int error_code, int next_eip_addend); > > +G_NORETURN void handle_unaligned_access(CPUX86State *env, vaddr vaddr, > > +MMUAccessType access_type, > > +uintptr_t retaddr); > > +#ifdef CONFIG_USER_ONLY > > +void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr, > > +MMUAccessType access_type, > > +bool maperr, uintptr_t ra); > > +void x86_cpu_record_sigbus(CPUState *cs, vaddr addr, > > + MMUAccessType access_type, uintptr_t ra); > > +#else > > +bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > > + MMUAccessType access_type, int mmu_idx, > > + bool probe, uintptr_t
[PULL 07/21] smbios: sanitize type from external type before checking have_fields_bitmap
test_bit uses header->type as an offset; if the file incorrectly specifies a type greater than 127, smbios_entry_add will read and write garbage. To fix this, just pass the smbios data through, assuming the user knows what to do. Reported by Coverity as CID 1487255. Signed-off-by: Paolo Bonzini --- hw/smbios/smbios.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index 60349ee402..4c9f664830 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -1205,13 +1205,15 @@ void smbios_entry_add(QemuOpts *opts, Error **errp) return; } -if (test_bit(header->type, have_fields_bitmap)) { -error_setg(errp, - "can't load type %d struct, fields already specified!", - header->type); -return; +if (header->type <= SMBIOS_MAX_TYPE) { +if (test_bit(header->type, have_fields_bitmap)) { +error_setg(errp, + "can't load type %d struct, fields already specified!", + header->type); +return; +} +set_bit(header->type, have_binfile_bitmap); } -set_bit(header->type, have_binfile_bitmap); if (header->type == 4) { smbios_type4_count++; -- 2.37.2
[PULL 08/21] tests: unit: simplify test-visitor-serialization list tests
test-visitor-serialization list tests is using an "if" to pick either the first element of the list or the next one. This was done presumably to mimic the code that creates the list, which has to fill in either the head pointer or the next pointer of the last element. However, the code in the insert phase is a pretty standard singly-linked list insertion, while the one in the visit phase looks weird and even looks at the first item twice: this is confusing because the test puts in 32 items and finishes with an assertion that i == 33. So, move the "else" step in a separate switch statement, and change the do...while loop to a while, because cur_head has already been initialized beforehand. Signed-off-by: Paolo Bonzini --- tests/unit/test-visitor-serialization.c | 157 +++- 1 file changed, 69 insertions(+), 88 deletions(-) diff --git a/tests/unit/test-visitor-serialization.c b/tests/unit/test-visitor-serialization.c index 907263d030..667e8fed82 100644 --- a/tests/unit/test-visitor-serialization.c +++ b/tests/unit/test-visitor-serialization.c @@ -427,131 +427,117 @@ static void test_primitive_lists(gconstpointer opaque) ops->deserialize((void **)_copy_ptr, serialize_data, visit_primitive_list, _abort); -i = 0; + +switch (pl_copy.type) { +case PTYPE_STRING: +cur_head = pl_copy.value.strings; +break; +case PTYPE_INTEGER: +cur_head = pl_copy.value.integers; +break; +case PTYPE_S8: +cur_head = pl_copy.value.s8_integers; +break; +case PTYPE_S16: +cur_head = pl_copy.value.s16_integers; +break; +case PTYPE_S32: +cur_head = pl_copy.value.s32_integers; +break; +case PTYPE_S64: +cur_head = pl_copy.value.s64_integers; +break; +case PTYPE_U8: +cur_head = pl_copy.value.u8_integers; +break; +case PTYPE_U16: +cur_head = pl_copy.value.u16_integers; +break; +case PTYPE_U32: +cur_head = pl_copy.value.u32_integers; +break; +case PTYPE_U64: +cur_head = pl_copy.value.u64_integers; +break; +case PTYPE_NUMBER: +cur_head = pl_copy.value.numbers; +break; +case PTYPE_BOOLEAN: +cur_head = pl_copy.value.booleans; +break; +default: +g_assert_not_reached(); +} /* compare our deserialized list of primitives to the original */ -do { +i = 0; +while (cur_head) { switch (pl_copy.type) { case PTYPE_STRING: { -strList *ptr; -if (cur_head) { -ptr = cur_head; -cur_head = ptr->next; -} else { -cur_head = ptr = pl_copy.value.strings; -} +strList *ptr = cur_head; +cur_head = ptr->next; g_assert_cmpstr(pt->value.string, ==, ptr->value); break; } case PTYPE_INTEGER: { -intList *ptr; -if (cur_head) { -ptr = cur_head; -cur_head = ptr->next; -} else { -cur_head = ptr = pl_copy.value.integers; -} +intList *ptr = cur_head; +cur_head = ptr->next; g_assert_cmpint(pt->value.integer, ==, ptr->value); break; } case PTYPE_S8: { -int8List *ptr; -if (cur_head) { -ptr = cur_head; -cur_head = ptr->next; -} else { -cur_head = ptr = pl_copy.value.s8_integers; -} +int8List *ptr = cur_head; +cur_head = ptr->next; g_assert_cmpint(pt->value.s8, ==, ptr->value); break; } case PTYPE_S16: { -int16List *ptr; -if (cur_head) { -ptr = cur_head; -cur_head = ptr->next; -} else { -cur_head = ptr = pl_copy.value.s16_integers; -} +int16List *ptr = cur_head; +cur_head = ptr->next; g_assert_cmpint(pt->value.s16, ==, ptr->value); break; } case PTYPE_S32: { -int32List *ptr; -if (cur_head) { -ptr = cur_head; -cur_head = ptr->next; -} else { -cur_head = ptr = pl_copy.value.s32_integers; -} +int32List *ptr = cur_head; +cur_head = ptr->next; g_assert_cmpint(pt->value.s32, ==, ptr->value); break; } case PTYPE_S64: { -int64List *ptr; -if (cur_head) { -ptr = cur_head; -cur_head = ptr->next; -} else { -cur_head = ptr = pl_copy.value.s64_integers; -} +int64List *ptr = cur_head; +cur_head = ptr->next;
[PULL 02/21] target/i386: Raise #GP on unaligned m128 accesses when required.
Many instructions which load/store 128-bit values are supposed to raise #GP when the memory operand isn't 16-byte aligned. This includes: - Instructions explicitly requiring memory alignment (Exceptions Type 1 in the "AVX and SSE Instruction Exception Specification" section of the SDM) - Legacy SSE instructions that load/store 128-bit values (Exceptions Types 2 and 4). This change sets MO_ALIGN_16 on 128-bit memory accesses that require 16-byte alignment. It adds cpu_record_sigbus and cpu_do_unaligned_access hooks that simulate a #GP exception in qemu-user and qemu-system, respectively. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/217 Reviewed-by: Richard Henderson Signed-off-by: Ricky Zhou Message-Id: <20220830034816.57091-2-ri...@rzhou.org> [Do not bother checking PREFIX_VEX, since AVX is not supported. - Paolo] Signed-off-by: Paolo Bonzini --- target/i386/tcg/excp_helper.c| 13 + target/i386/tcg/helper-tcg.h | 28 +++--- target/i386/tcg/sysemu/excp_helper.c | 8 ++ target/i386/tcg/tcg-cpu.c| 2 ++ target/i386/tcg/translate.c | 43 target/i386/tcg/user/excp_helper.c | 7 + 6 files changed, 72 insertions(+), 29 deletions(-) diff --git a/target/i386/tcg/excp_helper.c b/target/i386/tcg/excp_helper.c index c1ffa1c0ef..7c3c8dc7fe 100644 --- a/target/i386/tcg/excp_helper.c +++ b/target/i386/tcg/excp_helper.c @@ -140,3 +140,16 @@ G_NORETURN void raise_exception_ra(CPUX86State *env, int exception_index, { raise_interrupt2(env, exception_index, 0, 0, 0, retaddr); } + +G_NORETURN void handle_unaligned_access(CPUX86State *env, vaddr vaddr, +MMUAccessType access_type, +uintptr_t retaddr) +{ +/* + * Unaligned accesses are currently only triggered by SSE/AVX + * instructions that impose alignment requirements on memory + * operands. These instructions raise #GP(0) upon accessing an + * unaligned address. + */ +raise_exception_ra(env, EXCP0D_GPF, retaddr); +} diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h index 34167e2e29..cd1723389a 100644 --- a/target/i386/tcg/helper-tcg.h +++ b/target/i386/tcg/helper-tcg.h @@ -42,17 +42,6 @@ void x86_cpu_do_interrupt(CPUState *cpu); bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req); #endif -/* helper.c */ -#ifdef CONFIG_USER_ONLY -void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr, -MMUAccessType access_type, -bool maperr, uintptr_t ra); -#else -bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size, - MMUAccessType access_type, int mmu_idx, - bool probe, uintptr_t retaddr); -#endif - void breakpoint_handler(CPUState *cs); /* n must be a constant to be efficient */ @@ -78,6 +67,23 @@ G_NORETURN void raise_exception_err_ra(CPUX86State *env, int exception_index, int error_code, uintptr_t retaddr); G_NORETURN void raise_interrupt(CPUX86State *nenv, int intno, int is_int, int error_code, int next_eip_addend); +G_NORETURN void handle_unaligned_access(CPUX86State *env, vaddr vaddr, +MMUAccessType access_type, +uintptr_t retaddr); +#ifdef CONFIG_USER_ONLY +void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr, +MMUAccessType access_type, +bool maperr, uintptr_t ra); +void x86_cpu_record_sigbus(CPUState *cs, vaddr addr, + MMUAccessType access_type, uintptr_t ra); +#else +bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size, + MMUAccessType access_type, int mmu_idx, + bool probe, uintptr_t retaddr); +G_NORETURN void x86_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, +MMUAccessType access_type, +int mmu_idx, uintptr_t retaddr); +#endif /* cc_helper.c */ extern const uint8_t parity_table[256]; diff --git a/target/i386/tcg/sysemu/excp_helper.c b/target/i386/tcg/sysemu/excp_helper.c index 48feba7e75..796dc2a1f3 100644 --- a/target/i386/tcg/sysemu/excp_helper.c +++ b/target/i386/tcg/sysemu/excp_helper.c @@ -439,3 +439,11 @@ bool x86_cpu_tlb_fill(CPUState *cs, vaddr addr, int size, } return true; } + +G_NORETURN void x86_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, +MMUAccessType access_type, +int mmu_idx, uintptr_t retaddr) +{ +X86CPU *cpu = X86_CPU(cs); +handle_unaligned_access(>env, vaddr, access_type, retaddr); +} diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c index 6fdfdf9598..d3c2b8fb49 100644 ---
Re: [PATCH v3] 9pfs: use GHashTable for fid table
On Freitag, 9. September 2022 15:10:48 CEST Christian Schoenebeck wrote: > On Donnerstag, 8. September 2022 13:23:53 CEST Linus Heckemann wrote: > > The previous implementation would iterate over the fid table for > > lookup operations, resulting in an operation with O(n) complexity on > > the number of open files and poor cache locality -- for every open, > > stat, read, write, etc operation. > > > > This change uses a hashtable for this instead, significantly improving > > the performance of the 9p filesystem. The runtime of NixOS's simple > > installer test, which copies ~122k files totalling ~1.8GiB from 9p, > > decreased by a factor of about 10. > > > > Signed-off-by: Linus Heckemann > > Reviewed-by: Philippe Mathieu-Daudé > > Reviewed-by: Greg Kurz > > --- > > Queued on 9p.next: > https://github.com/cschoenebeck/qemu/commits/9p.next > > I retained the BUG_ON() in get_fid(), Greg had a point there that continuing > to work on a clunked fid would still be a bug. > > I also added the suggested TODO comment for g_hash_table_steal_extended(), > the actual change would be outside the scope of this patch. > > And finally I gave this patch a whirl, and what can I say: that's just sick! > Compiling sources with 9p is boosted by around factor 6..7 here! And > running 9p as root fs also no longer feels sluggish as before. I mean I > knew that this fid list traversal performance issue existed and had it on > my TODO list, but the actual impact exceeded my expectation by far. Linus, there is still something cheesy. After more testing, at a certain point running the VM, the terminal is spilled with this message: GLib: g_hash_table_iter_next: assertion 'ri->version == ri->hash_table->version' failed Looking at the glib sources, I think this warning means the iterator got invalidated. Setting a breakpoint at glib function g_return_if_fail_warning I got: Thread 1 "qemu-system-x86" hit Breakpoint 1, 0x77aa9d80 in g_return_if_fail_warning () from /lib/x86_64-linux-gnu/libglib-2.0.so.0 (gdb) bt #0 0x77aa9d80 in g_return_if_fail_warning () at /lib/x86_64-linux-gnu/libglib-2.0.so.0 #1 0x77a8ea18 in g_hash_table_iter_next () at /lib/x86_64-linux-gnu/libglib-2.0.so.0 #2 0x55998a7a in v9fs_mark_fids_unreclaim (pdu=0x57a34c90, path=0x7ffba8ceff30) at ../hw/9pfs/9p.c:528 #3 0x5599f7a0 in v9fs_unlinkat (opaque=0x57a34c90) at ../hw/9pfs/9p.c:3170 #4 0x5606dc4b in coroutine_trampoline (i0=1463900480, i1=21845) at ../util/coroutine-ucontext.c:177 #5 0x77749d40 in __start_context () at /lib/x86_64-linux-gnu/libc.so.6 #6 0x7fffd5f0 in () #7 0x in () (gdb) The while loop in v9fs_mark_fids_unreclaim() holds the hash table iterator while the hash table is modified during the loop. Would you please fix this? If you do, please use my already queued patch version as basis. Best regards, Christian Schoenebeck
[PULL 06/21] coverity: put NUBus under m68k component
It is only used by the Q800 emulation, so put it under that architecture. Signed-off-by: Paolo Bonzini --- scripts/coverity-scan/COMPONENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index fc1608932e..0e6ab4936e 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -22,7 +22,7 @@ i386 ~ (/qemu)?((/include)?/hw/i386/.*|/target/i386/.*|/hw/intc/[^/]*apic[^/]*\.c) m68k - ~ (/qemu)?((/include)?/hw/m68k/.*|/target/m68k/.*|(/include)?/hw(/.*)?/mcf.*) + ~ (/qemu)?((/include)?/hw/m68k/.*|/target/m68k/.*|(/include)?/hw(/.*)?/mcf.*|(/include)?/hw/nubus/.*) microblaze ~ (/qemu)?((/include)?/hw/microblaze/.*|/target/microblaze/.*) -- 2.37.2
Re: [PATCH v3 1/2] target/i386: Read 8 bytes from cvttps2pi/cvtps2pi memory operands
Hi, I think this is broken for big endian systems because ldq expects a pointer to xmm_t0.L(0) while ldo expects a pointer xmm_t0. I will fix the bug in my new AVX decoder though, where it is also present. So thanks for the report! Paolo On Tue, Aug 30, 2022 at 5:48 AM Ricky Zhou wrote: > > Before this change, emulation of cvttps2pi and cvtps2pi instructions > would read 16 bytes of memory instead of 8. The SDM states that > cvttps2pi takes a 64-bit memory location. The documentation for cvtps2pi > claims that it takes a a 128-bit memory location, but as with cvttps2pi, > the operand is written as xmm/m64. I double-checked on real hardware > that both of these instructions only read 8 bytes. > > Reviewed-by: Richard Henderson > Signed-off-by: Ricky Zhou > --- > target/i386/tcg/translate.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c > index b7972f0ff5..3ba5f76156 100644 > --- a/target/i386/tcg/translate.c > +++ b/target/i386/tcg/translate.c > @@ -3621,7 +3621,11 @@ static void gen_sse(CPUX86State *env, DisasContext *s, > int b, > if (mod != 3) { > gen_lea_modrm(env, s, modrm); > op2_offset = offsetof(CPUX86State,xmm_t0); > -gen_ldo_env_A0(s, op2_offset); > +if (b1) { > +gen_ldo_env_A0(s, op2_offset); > +} else { > +gen_ldq_env_A0(s, op2_offset); > +} > } else { > rm = (modrm & 7) | REX_B(s); > op2_offset = offsetof(CPUX86State,xmm_regs[rm]); > -- > 2.37.2 >
[PULL 00/21] Misc patches for 2022-09-19
The following changes since commit d29201ff34a135cdfc197f4413c1c5047e4f58bb: Merge tag 'pull-hmp-20220915a' of https://gitlab.com/dagrh/qemu into staging (2022-09-17 10:31:11 -0400) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to df22fbb751dc72f321218c3fb192730a47ad59a9: qboot: update to latest submodule (2022-09-19 15:40:51 +0200) * add help option for -audio and -audiodev * another missing memory barrier for dirty pages * target/i386: Raise #GP on unaligned m128 accesses * coverity fixes + improvements to components * add MMX and 3DNow! tests * SSE4a fixes * target/i386: TCG translation cleanups * update qboot submodule Claudio Fontana (1): audio: add help option for -audio and -audiodev Paolo Bonzini (20): KVM: use store-release to mark dirty pages as harvested target/i386: Raise #GP on unaligned m128 accesses when required. kvm: fix memory leak on failure to read stats descriptors spapr_pci: fix leak in spapr_phb_vfio_get_loc_code coverity: add new RISC-V component coverity: put NUBus under m68k component smbios: sanitize type from external type before checking have_fields_bitmap tests: unit: simplify test-visitor-serialization list tests tests: test-qga: close socket on failure to connect tests: unit: add NULL-pointer check tests/tcg: i386: fix typos in 3DNow! instructions tests/tcg: i386: add MMX and 3DNow! tests tests/tcg: refine MMX support in SSE tests tests/tcg: remove old SSE tests target/i386: correctly mask SSE4a bit indices in register operands target/i386: fix INSERTQ implementation target/i386: REPZ and REPNZ are mutually exclusive target/i386: introduce insn_get_addr build: remove extra parentheses causing missing rebuilds qboot: update to latest submodule Makefile| 2 +- accel/kvm/kvm-all.c | 27 +- audio/audio.c | 19 ++ audio/audio.h | 1 + hw/ppc/spapr_pci.c | 1 + hw/smbios/smbios.c | 14 +- qemu-options.hx | 10 +- roms/qboot | 2 +- scripts/coverity-scan/COMPONENTS.md | 5 +- softmmu/vl.c| 9 +- target/i386/ops_sse.h | 12 +- target/i386/ops_sse_header.h| 2 +- target/i386/tcg/excp_helper.c | 13 + target/i386/tcg/helper-tcg.h| 28 +- target/i386/tcg/sysemu/excp_helper.c| 8 + target/i386/tcg/tcg-cpu.c | 2 + target/i386/tcg/translate.c | 95 -- target/i386/tcg/user/excp_helper.c | 7 + tests/tcg/i386/Makefile.target | 24 +- tests/tcg/i386/test-3dnow.c | 3 + tests/tcg/i386/test-avx.py | 33 +- tests/tcg/i386/test-i386.c | 573 tests/tcg/i386/test-mmx.c | 315 ++ tests/tcg/i386/test-mmx.py | 244 ++ tests/tcg/i386/x86.csv | 4 +- tests/tcg/x86_64/Makefile.target| 1 - tests/unit/check-block-qdict.c | 2 +- tests/unit/test-qga.c | 1 + tests/unit/test-visitor-serialization.c | 157 - 29 files changed, 870 insertions(+), 744 deletions(-) create mode 100644 tests/tcg/i386/test-3dnow.c create mode 100644 tests/tcg/i386/test-mmx.c create mode 100755 tests/tcg/i386/test-mmx.py -- 2.37.2
[PATCH 3/3] hw/arm/aspeed: g220a: Add host-power device
Add power-button/power-good gpio connect between g220a BMC machind(soc gpio) and host. Tested: In qemu, use g220a image ~# ipmitool power status Chassis Power is off ~# ipmitool power on Chassis Power Control: Up/On ~# ipmitool power status Chassis Power is on ~# ipmitool power off Chassis Power Control: Down/Off ~# ipmitool power status Chassis Power is off Signed-off-by: Jian Zhang --- hw/arm/aspeed.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 952fa11ca2..80a98b8d74 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -28,6 +28,7 @@ #include "hw/qdev-clock.h" #include "sysemu/sysemu.h" #include "hw/arm/fby35.h" +#include "hw/misc/host_power.h" static struct arm_boot_info aspeed_board_binfo = { .board_id = -1, /* device-tree-only board */ @@ -723,6 +724,24 @@ static void g220a_bmc_i2c_init(AspeedMachineState *bmc) }; smbus_eeprom_init_one(aspeed_i2c_get_bus(>i2c, 4), 0x57, eeprom_buf); + +/* Add a host-power device */ +HostPowerState *power = host_power_create_simple(OBJECT(bmc)); + +/* + * connect the power button(in) to soc(out) + * the power button in g220a is 215 + */ +qdev_connect_gpio_out(DEVICE(>soc.gpio), 215, + qdev_get_gpio_in_named(DEVICE(power), + "power-button", 0)); + +/* + * connect the power good signal(out) to soc(in) + * the power good in g220a is 209 + */ +qdev_connect_gpio_out_named(DEVICE(power), "power-good", 0, +qdev_get_gpio_in(DEVICE(>soc.gpio), 209)); } static void aspeed_eeprom_init(I2CBus *bus, uint8_t addr, uint32_t rsize) -- 2.25.1
[PATCH 2/3] hw/misc/host_power: Add a simple host power device
This Host Power device privide a simple power control logic for a host, like use a bmc to control the power of a host. This device has 2 gpio, one is input named "button", another gpio is output named "power-good", when button have a falling edge, invert the "power-good" gpio. Signed-off-by: Jian Zhang --- MAINTAINERS | 2 + hw/arm/Kconfig | 1 + hw/misc/Kconfig | 3 + hw/misc/host_power.c | 105 +++ hw/misc/meson.build | 1 + include/hw/misc/host_power.h | 41 ++ 6 files changed, 153 insertions(+) create mode 100644 hw/misc/host_power.c create mode 100644 include/hw/misc/host_power.h diff --git a/MAINTAINERS b/MAINTAINERS index 472fbf4f42..5a27a78985 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1068,6 +1068,8 @@ F: tests/qtest/*aspeed* F: hw/arm/fby35.c F: hw/misc/fby35_sb_cpld.c F: hw/misc/intel_me.c +F: include/hw/misc/host_power.h +F: hw/misc/host_power.c NRF51 M: Joel Stanley diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 23330cca52..f6fa364ab7 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -455,6 +455,7 @@ config ASPEED_SOC select EMC141X select UNIMP select LED +select HOST_POWER select PMBUS select MAX31785 diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig index d0e691990a..e0b168ec1d 100644 --- a/hw/misc/Kconfig +++ b/hw/misc/Kconfig @@ -147,6 +147,9 @@ config UNIMP config LED bool +config HOST_POWER +bool + config MAC_VIA bool select MOS6522 diff --git a/hw/misc/host_power.c b/hw/misc/host_power.c new file mode 100644 index 00..18d2573d5e --- /dev/null +++ b/hw/misc/host_power.c @@ -0,0 +1,105 @@ +/* + * QEMU single Host Power device + * + * Copyright (C) 2022 Jian Zhang + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "migration/vmstate.h" +#include "hw/qdev-properties.h" +#include "hw/irq.h" +#include "hw/misc/host_power.h" +#include "trace.h" + +static void power_control(HostPowerState *s, bool on) +{ +if (on) { +qemu_set_irq(s->power_good, 1); +} else { +qemu_set_irq(s->power_good, 0); +} +s->power_status = on; +} + +static void power_button_handler(void *opaque, int line, int new_state) +{ +HostPowerState *s = HOST_POWER(opaque); + +assert(line == 0); + +if (new_state == 0) { +/* falling edge, reverse the power status */ +if (s->power_status == 0) { +power_control(s, true); +} else { +power_control(s, false); +} +} +} + +static void host_power_reset(DeviceState *dev) +{ +HostPowerState *s = HOST_POWER(dev); +s->power_status = false; +} + +static const VMStateDescription vmstate_host_power = { +.name = TYPE_HOST_POWER, +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_END_OF_LIST() +} +}; + +static void host_power_realize(DeviceState *dev, Error **errp) +{ +HostPowerState *s = HOST_POWER(dev); +s->power_status = false; + +/* init a power button gpio as input pin */ +qdev_init_gpio_in_named(dev, power_button_handler, "power-button", 1); + +/* init a power good gpio as output pin */ +qdev_init_gpio_out_named(dev, &(s->power_good), "power-good", 1); +} + +static void host_power_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); + +dc->desc = "Host Power"; +dc->vmsd = _host_power; +dc->reset = host_power_reset; +dc->realize = host_power_realize; +set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories); +} + +static const TypeInfo host_power_info = { +.name = TYPE_HOST_POWER, +.parent = TYPE_DEVICE, +.instance_size = sizeof(HostPowerState), +.class_init = host_power_class_init +}; + +static void host_power_register_types(void) +{ +type_register_static(_power_info); +} + +type_init(host_power_register_types) + +HostPowerState *host_power_create_simple(Object *parentobj) +{ +static const char *name = "host-power"; +DeviceState *dev; + +dev = qdev_new(TYPE_HOST_POWER); + +object_property_add_child(parentobj, name, OBJECT(dev)); +qdev_realize_and_unref(dev, NULL, _fatal); + +return HOST_POWER(dev); +} diff --git a/hw/misc/meson.build b/hw/misc/meson.build index 87d65c16a6..be14c1399a 100644 --- a/hw/misc/meson.build +++ b/hw/misc/meson.build @@ -9,6 +9,7 @@ softmmu_ss.add(when: 'CONFIG_SGA', if_true: files('sga.c')) softmmu_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c')) softmmu_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c')) softmmu_ss.add(when: 'CONFIG_LED', if_true: files('led.c')) +softmmu_ss.add(when: 'CONFIG_HOST_POWER', if_true: files('host_power.c')) softmmu_ss.add(when: 'CONFIG_PVPANIC_COMMON', if_true: files('pvpanic.c')) # ARM devices diff --git a/include/hw/misc/host_power.h b/include/hw/misc/host_power.h
[PATCH 1/3] hw/gpio/aspeed_gpio: Add gpios in/out init
Add gpios in/out init for aspeed gpio to add the ability to connect to other gpio devices. Based the qdev-core.h comments, If you want to connect a GPIO to other devices, you need to call qdev_init_gpio_in() or qdev_init_gpio_out(). ``` For input gpios: * * Outbound GPIO lines can be connected to any qemu_irq, but the common * case is connecting them to another device's inbound GPIO line, using * the qemu_irq returned by qdev_get_gpio_in() or qdev_get_gpio_in_named(). For output gpios: * This function is intended to be used by board code or SoC "container" * device models to wire up the GPIO lines; usually the return value * will be passed to qdev_connect_gpio_out() or a similar function to * connect another device's output GPIO line to this input. ``` Signed-off-by: Jian Zhang --- hw/gpio/aspeed_gpio.c | 17 + 1 file changed, 17 insertions(+) diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index e99c4c6329..616ec8db52 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -1018,6 +1018,17 @@ static void aspeed_gpio_reset(DeviceState *dev) memset(s->sets, 0, sizeof(s->sets)); } +static void aspeed_gpio_set(void *opaque, int line, int new_state) +{ +AspeedGPIOState *s = ASPEED_GPIO(opaque); +uint32_t set_idx, pin; + +set_idx = line / ASPEED_GPIOS_PER_SET; +pin = line % ASPEED_GPIOS_PER_SET; + +aspeed_gpio_set_pin_level(s, set_idx, pin, new_state); +} + static void aspeed_gpio_realize(DeviceState *dev, Error **errp) { AspeedGPIOState *s = ASPEED_GPIO(dev); @@ -1042,6 +1053,12 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp) memory_region_init_io(>iomem, OBJECT(s), _gpio_ops, s, TYPE_ASPEED_GPIO, 0x800); +/* TODO: Maybe could in named, not anonymous is better */ +qdev_init_gpio_out(dev, >gpios[0][0], + ASPEED_GPIO_MAX_NR_SETS * ASPEED_GPIOS_PER_SET); +qdev_init_gpio_in(dev, aspeed_gpio_set, + ASPEED_GPIO_MAX_NR_SETS * ASPEED_GPIOS_PER_SET); + sysbus_init_mmio(sbd, >iomem); } -- 2.25.1
[PATCH 0/3] Add a host power device
This patchset adds a host power device and added it into the g220a mahcine. The BMC have a important is to control the power of the host, usually it is nessary in a hardware platform. The BMC(soc) usually had a output pin to control the power of the host, and a input pin to get the power status of the host. The host power device is a generic device to simulate the host power, accept the power control command from the BMC and report the power status. Test on the g220a machine, the host power control command can be simply work. Jian Zhang (3): hw/gpio/aspeed_gpio: Add gpios in/out init hw/misc/host_power: Add a simple host power device hw/arm/aspeed: g220a: Add host-power device MAINTAINERS | 2 + hw/arm/Kconfig | 1 + hw/arm/aspeed.c | 19 +++ hw/gpio/aspeed_gpio.c| 17 ++ hw/misc/Kconfig | 3 + hw/misc/host_power.c | 105 +++ hw/misc/meson.build | 1 + include/hw/misc/host_power.h | 41 ++ 8 files changed, 189 insertions(+) create mode 100644 hw/misc/host_power.c create mode 100644 include/hw/misc/host_power.h -- 2.25.1
Re: [PATCH v7 for-7.2 00/15] block: cleanup backing and file handling
ping. Seems, all patches are reviewed On 7/26/22 23:11, Vladimir Sementsov-Ogievskiy wrote: Hi all! That's the first part of "[PATCH v5 00/45] Transactional block-graph modifying API", updated and is fully reviewed by Hanna. v7: add r-bs and rebase on master Vladimir Sementsov-Ogievskiy (15): block: BlockDriver: add .filtered_child_is_backing field block: introduce bdrv_open_file_child() helper block/blklogwrites: don't care to remove bs->file child on failure test-bdrv-graph-mod: update test_parallel_perm_update test case tests-bdrv-drain: bdrv_replace_test driver: declare supports_backing test-bdrv-graph-mod: fix filters to be filters block: document connection between child roles and bs->backing/bs->file block/snapshot: stress that we fallback to primary child Revert "block: Let replace_child_noperm free children" Revert "block: Let replace_child_tran keep indirect pointer" Revert "block: Restructure remove_file_or_backing_child()" Revert "block: Pass BdrvChild ** to replace_child_noperm" block: Manipulate bs->file / bs->backing pointers in .attach/.detach block/snapshot: drop indirection around bdrv_snapshot_fallback_ptr block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child block.c| 435 ++--- block/blkdebug.c | 9 +- block/blklogwrites.c | 11 +- block/blkreplay.c | 7 +- block/blkverify.c | 9 +- block/bochs.c | 7 +- block/cloop.c | 7 +- block/commit.c | 1 + block/copy-before-write.c | 9 +- block/copy-on-read.c | 9 +- block/crypto.c | 11 +- block/dmg.c| 7 +- block/filter-compress.c| 8 +- block/mirror.c | 1 + block/parallels.c | 7 +- block/preallocate.c| 9 +- block/qcow.c | 6 +- block/qcow2.c | 8 +- block/qed.c| 8 +- block/raw-format.c | 4 +- block/replication.c| 8 +- block/snapshot-access.c| 6 +- block/snapshot.c | 59 ++-- block/throttle.c | 8 +- block/vdi.c| 7 +- block/vhdx.c | 7 +- block/vmdk.c | 7 +- block/vpc.c| 7 +- include/block/block-common.h | 39 +++ include/block/block-global-state.h | 3 + include/block/block_int-common.h | 29 +- tests/unit/test-bdrv-drain.c | 11 +- tests/unit/test-bdrv-graph-mod.c | 104 --- 33 files changed, 389 insertions(+), 479 deletions(-) -- Best regards, Vladimir
Re: [PATCH v2 00/11] iotests: use vm.cmd()
ping On 6/6/22 10:27, Vladimir Sementsov-Ogievskiy wrote: Hi all! Let's get rid of pattern result = self.vm.qmp(...) self.assert_qmp(result, 'return', {}) And switch to just self.vm.cmd(...) Supersedes: <20220408170214.45585-1-vsement...@openvz.org> ([RFC 0/2] introduce QEMUMachind.cmd()) Vladimir Sementsov-Ogievskiy (11): python: rename QEMUMonitorProtocol.cmd() to cmd_raw() python/qemu: rename command() to cmd() python/machine.py: upgrade vm.cmd() method iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine. iotests: add some missed checks of qmp result iotests: refactor some common qmp result checks into generic pattern iotests: drop some occasional semicolons iotests: drop some extra ** in qmp() call iotests.py: pause_job(): drop return value tests/vm/basevm.py: use cmd() instead of qmp() python: use vm.cmd() instead of vm.qmp() where appropriate docs/devel/testing.rst| 10 +- python/qemu/machine/machine.py| 20 +- python/qemu/qmp/legacy.py | 10 +- python/qemu/qmp/qmp_shell.py | 13 +- python/qemu/utils/qemu_ga_client.py | 2 +- python/qemu/utils/qom.py | 8 +- python/qemu/utils/qom_common.py | 2 +- python/qemu/utils/qom_fuse.py | 6 +- scripts/cpu-x86-uarch-abi.py | 8 +- scripts/device-crash-test | 8 +- scripts/render_block_graph.py | 8 +- tests/avocado/avocado_qemu/__init__.py| 4 +- tests/avocado/cpu_queries.py | 4 +- tests/avocado/hotplug_cpu.py | 10 +- tests/avocado/info_usernet.py | 4 +- tests/avocado/machine_arm_integratorcp.py | 6 +- tests/avocado/machine_m68k_nextcube.py| 4 +- tests/avocado/machine_mips_malta.py | 6 +- tests/avocado/machine_s390_ccw_virtio.py | 28 +- tests/avocado/migration.py| 10 +- tests/avocado/pc_cpu_hotplug_props.py | 2 +- tests/avocado/version.py | 4 +- tests/avocado/virtio_check_params.py | 6 +- tests/avocado/virtio_version.py | 4 +- tests/avocado/vnc.py | 16 +- tests/avocado/x86_cpu_model_versions.py | 10 +- tests/migration/guestperf/engine.py | 150 +++--- tests/qemu-iotests/030| 168 +++--- tests/qemu-iotests/040| 171 +++ tests/qemu-iotests/041| 482 -- tests/qemu-iotests/045| 15 +- tests/qemu-iotests/055| 62 +-- tests/qemu-iotests/056| 77 ++- tests/qemu-iotests/093| 42 +- tests/qemu-iotests/118| 225 tests/qemu-iotests/124| 102 ++-- tests/qemu-iotests/129| 14 +- tests/qemu-iotests/132| 5 +- tests/qemu-iotests/139| 45 +- tests/qemu-iotests/147| 30 +- tests/qemu-iotests/151| 56 +- tests/qemu-iotests/152| 8 +- tests/qemu-iotests/155| 55 +- tests/qemu-iotests/165| 8 +- tests/qemu-iotests/196| 3 +- tests/qemu-iotests/205| 6 +- tests/qemu-iotests/218| 105 ++-- tests/qemu-iotests/245| 245 - tests/qemu-iotests/256| 34 +- tests/qemu-iotests/257| 36 +- tests/qemu-iotests/264| 31 +- tests/qemu-iotests/281| 21 +- tests/qemu-iotests/295| 16 +- tests/qemu-iotests/296| 21 +- tests/qemu-iotests/298| 13 +- tests/qemu-iotests/300| 54 +- tests/qemu-iotests/iotests.py | 18 +- .../tests/export-incoming-iothread| 6 +- .../qemu-iotests/tests/graph-changes-while-io | 6 +- tests/qemu-iotests/tests/image-fleecing | 3 +- .../tests/migrate-bitmaps-postcopy-test | 31 +- tests/qemu-iotests/tests/migrate-bitmaps-test | 45 +- .../qemu-iotests/tests/migrate-during-backup | 41 +- .../qemu-iotests/tests/migration-permissions | 9 +- .../tests/mirror-ready-cancel-error | 74 ++- tests/qemu-iotests/tests/mirror-top-perms | 16 +- tests/qemu-iotests/tests/nbd-multiconn| 12 +- tests/qemu-iotests/tests/reopen-file | 3 +- .../qemu-iotests/tests/stream-error-on-reset | 6 +- tests/vm/basevm.py| 4 +- 70 files changed,
Travis CI webhook returns HTTP 500
GitLab sends qemu.git push event webhooks to Travis CI. Recently the webhooks have been failing with HTTP 500 Internal Server Error. Do you know how to resolve this or who configured Travis CI webhooks for QEMU? Thanks, Stefan
Re: [PULL 0/9] loongarch-to-apply queue
Hi Song Gao, Please push your GPG public key to a key server using "gpg --send-keys 0x40A2FFF239263EDF". That way others can search for and download your public key. Thanks, Stefan
Re: [PATCH v4 for 7.2 00/22] virtio-gpio and various virtio cleanups
On Fri, Sep 16, 2022 at 07:51:40AM +0100, Alex Bennée wrote: > > Alex Bennée writes: > > > Hi, > > > > This is an update to the previous series which fixes the last few > > niggling CI failures I was seeing. > > > >Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups > >Date: Tue, 26 Jul 2022 20:21:29 +0100 > >Message-Id: <20220726192150.2435175-1-alex.ben...@linaro.org> > > > > The CI failures were tricky to track down because they didn't occur > > locally but after patching to dump backtraces they all seem to involve > > updates to virtio_set_status() as the machine was torn down. I think > > patch that switches all users to use virtio_device_started() along > > with consistent checking of vhost_dev->started stops this from > > happening. The clean-up seems worthwhile in reducing boilerplate > > anyway. > > > > The following patches still need review: > > > > - tests/qtest: enable tests for virtio-gpio > > - tests/qtest: add a get_features op to vhost-user-test > > - tests/qtest: implement stub for VHOST_USER_GET_CONFIG > > - tests/qtest: add assert to catch bad features > > - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES > > - tests/qtest: catch unhandled vhost-user messages > > - tests/qtest: use qos_printf instead of g_test_message > > - tests/qtest: pass stdout/stderr down to subtests > > - hw/virtio: move vhd->started check into helper and add FIXME > > - hw/virtio: move vm_running check to virtio_device_started > > - hw/virtio: add some vhost-user trace events > > - hw/virtio: log potentially buggy guest drivers > > - hw/virtio: fix some coding style issues > > - include/hw: document vhost_dev feature life-cycle > > - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE > > - hw/virtio: fix vhost_user_read tracepoint > > - hw/virtio: handle un-configured shutdown in virtio-pci > > - hw/virtio: gracefully handle unset vhost_dev vdev > > - hw/virtio: incorporate backend features in features > > > Ping? Who are you pinging? Only qemu-devel is on To and there are a bunch of people on Cc. Stefan signature.asc Description: PGP signature
Re: [PATCH v6 2/2] i386: Add notify VM exit support
On Mon, Sep 19, 2022 at 01:46:38PM +0800, Chenyi Qiang wrote: > > > On 9/17/2022 5:57 AM, Peter Xu wrote: > > On Thu, Sep 15, 2022 at 05:28:39PM +0800, Chenyi Qiang wrote: > > > There are cases that malicious virtual machine can cause CPU stuck (due > > > to event windows don't open up), e.g., infinite loop in microcode when > > > nested #AC (CVE-2015-5307). No event window means no event (NMI, SMI and > > > IRQ) can be delivered. It leads the CPU to be unavailable to host or > > > other VMs. Notify VM exit is introduced to mitigate such kind of > > > attacks, which will generate a VM exit if no event window occurs in VM > > > non-root mode for a specified amount of time (notify window). > > > > > > A new KVM capability KVM_CAP_X86_NOTIFY_VMEXIT is exposed to user space > > > so that the user can query the capability and set the expected notify > > > window when creating VMs. The format of the argument when enabling this > > > capability is as follows: > > >Bit 63:32 - notify window specified in qemu command > > >Bit 31:0 - some flags (e.g. KVM_X86_NOTIFY_VMEXIT_ENABLED is set to > > >enable the feature.) > > > > > > Because there are some concerns, e.g. a notify VM exit may happen with > > > VM_CONTEXT_INVALID set in exit qualification (no cases are anticipated > > > that would set this bit), which means VM context is corrupted. To avoid > > > the false positive and a well-behaved guest gets killed, make this > > > feature disabled by default. Users can enable the feature by a new > > > machine property: > > > qemu -machine notify_vmexit=on,notify_window=0 ... > > > > > > Note that notify_window is only valid when notify_vmexit is on. The valid > > > range of notify_window is non-negative. It is even safe to set it to zero > > > since there's an internal hardware threshold to be added to ensure no > > > false > > > positive. > > > > > > A new KVM exit reason KVM_EXIT_NOTIFY is defined for notify VM exit. If > > > it happens with VM_INVALID_CONTEXT, hypervisor exits to user space to > > > inform the fatal case. Then user space can inject a SHUTDOWN event to > > > the target vcpu. This is implemented by injecting a sythesized triple > > > fault event. > > > > > > Signed-off-by: Chenyi Qiang > > > --- > > > hw/i386/x86.c | 45 +++ > > > include/hw/i386/x86.h | 5 + > > > qemu-options.hx | 10 +- > > > target/i386/kvm/kvm.c | 28 +++ > > > 4 files changed, 87 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/i386/x86.c b/hw/i386/x86.c > > > index 050eedc0c8..1eccbd3deb 100644 > > > --- a/hw/i386/x86.c > > > +++ b/hw/i386/x86.c > > > @@ -1379,6 +1379,37 @@ static void machine_set_sgx_epc(Object *obj, > > > Visitor *v, const char *name, > > > qapi_free_SgxEPCList(list); > > > } > > > +static bool x86_machine_get_notify_vmexit(Object *obj, Error **errp) > > > +{ > > > +X86MachineState *x86ms = X86_MACHINE(obj); > > > + > > > +return x86ms->notify_vmexit; > > > +} > > > + > > > +static void x86_machine_set_notify_vmexit(Object *obj, bool value, Error > > > **errp) > > > +{ > > > +X86MachineState *x86ms = X86_MACHINE(obj); > > > + > > > +x86ms->notify_vmexit = value; > > > +} > > > + > > > +static void x86_machine_get_notify_window(Object *obj, Visitor *v, > > > +const char *name, void *opaque, Error > > > **errp) > > > +{ > > > +X86MachineState *x86ms = X86_MACHINE(obj); > > > +uint32_t notify_window = x86ms->notify_window; > > > + > > > +visit_type_uint32(v, name, _window, errp); > > > +} > > > + > > > +static void x86_machine_set_notify_window(Object *obj, Visitor *v, > > > + const char *name, void *opaque, Error > > > **errp) > > > +{ > > > +X86MachineState *x86ms = X86_MACHINE(obj); > > > + > > > +visit_type_uint32(v, name, >notify_window, errp); > > > +} > > > + > > > static void x86_machine_initfn(Object *obj) > > > { > > > X86MachineState *x86ms = X86_MACHINE(obj); > > > @@ -1392,6 +1423,8 @@ static void x86_machine_initfn(Object *obj) > > > x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8); > > > x86ms->bus_lock_ratelimit = 0; > > > x86ms->above_4g_mem_start = 4 * GiB; > > > +x86ms->notify_vmexit = false; > > > +x86ms->notify_window = 0; > > > } > > > static void x86_machine_class_init(ObjectClass *oc, void *data) > > > @@ -1461,6 +1494,18 @@ static void x86_machine_class_init(ObjectClass > > > *oc, void *data) > > > NULL, NULL); > > > object_class_property_set_description(oc, "sgx-epc", > > > "SGX EPC device"); > > > + > > > +object_class_property_add(oc, X86_MACHINE_NOTIFY_WINDOW, "uint32_t", > > > + x86_machine_get_notify_window, > > > + x86_machine_set_notify_window, NULL, NULL); > > > +
Re: [kvm-unit-tests PATCH v4 07/12] arm: pmu: Basic event counter Tests
On Mon, Sep 19, 2022 at 10:30:01PM +0800, Zenghui Yu wrote: > Hi Eric, > > A few comments when looking through the PMU test code (2 years after > the series was merged). Yes, these patches were merged long ago. Now you need to send patches, not comments. Thanks, drew
Re: [PATCH v3 2/5] tests/x86: Add 'q35' machine type to ivshmem-test
On 9/19/22 16:13, Denis V. Lunev wrote: On 9/15/22 15:14, Michael Labiuk wrote: diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c index 9611d05eb5..0f9755abc6 100644 --- a/tests/qtest/ivshmem-test.c +++ b/tests/qtest/ivshmem-test.c @@ -378,6 +378,32 @@ static void test_ivshmem_server(void) close(thread.pipe[0]); } +static void device_del(QTestState *qtest, const char *id) +{ + QDict *resp; + + resp = qtest_qmp(qtest, + "{'execute': 'device_del'," + " 'arguments': { 'id': %s } }", id); + + g_assert(qdict_haskey(resp, "return")); + qobject_unref(resp); +} hmmm, why do we need this helper if it is not used anywhere in next and this patches? it is also unclear to me why don't we do 'device_del' for other archs. May be this is to be clarified in the patch description or worth additional patch. device_del() used instead of qpci_unplug_acpi_device_test() because unplug_acpi is supported for x86 i440fx only. Also "DEVICE_DELETED" will not being emitted for 'q35' pci-e device without support from guest side. These are the reasons for custom unplugging function.
RE: [PATCH v6 1/2] Update AVX512 support for xbzrle_encode_buffer
Hi, All, This is a "ping" email~. It seems that my patch has been ignored. So I "ping" this patchset. Link for the patch: https://lore.kernel.org/qemu-devel/20220826095719.2887535-2-ling1...@intel.com/ Best Regards Ling -Original Message- From: Xu, Ling1 Sent: Friday, August 26, 2022 5:57 PM To: qemu-devel@nongnu.org Cc: quint...@redhat.com; dgilb...@redhat.com; Xu, Ling1 ; Zhao, Zhou ; Jin, Jun I Subject: [PATCH v6 1/2] Update AVX512 support for xbzrle_encode_buffer This commit updates code of avx512 support for xbzrle_encode_buffer function to accelerate xbzrle encoding speed. Runtime check of avx512 support and benchmark for this feature are added. Compared with C version of xbzrle_encode_buffer function, avx512 version can achieve 50%-70% performance improvement on benchmarking. In addition, if dirty data is randomly located in 4K page, the avx512 version can achieve almost 140% performance gain. Signed-off-by: ling xu Co-authored-by: Zhou Zhao Co-authored-by: Jun Jin --- meson.build| 16 ++ meson_options.txt | 2 + migration/ram.c| 34 +++-- migration/xbzrle.c | 124 + migration/xbzrle.h | 4 ++ 5 files changed, 177 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 20fddbd707..5d4b82d7f3 100644 --- a/meson.build +++ b/meson.build @@ -2264,6 +2264,22 @@ config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \ int main(int argc, char *argv[]) { return bar(argv[0]); } '''), error_message: 'AVX512F not available').allowed()) +config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \ + .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot +enable AVX512BW') \ + .require(cc.links(''' +#pragma GCC push_options +#pragma GCC target("avx512bw") +#include +#include +static int bar(void *a) { + + __m512i *x = a; + __m512i res= _mm512_abs_epi8(*x); + return res[1]; +} +int main(int argc, char *argv[]) { return bar(argv[0]); } '''), + error_message: 'AVX512BW not available').allowed()) + have_pvrdma = get_option('pvrdma') \ .require(rdma.found(), error_message: 'PVRDMA requires OpenFabrics libraries') \ .require(cc.compiles(gnu_source_prefix + ''' diff --git a/meson_options.txt b/meson_options.txt index e58e158396..07194bf680 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -104,6 +104,8 @@ option('avx2', type: 'feature', value: 'auto', description: 'AVX2 optimizations') option('avx512f', type: 'feature', value: 'disabled', description: 'AVX512F optimizations') +option('avx512bw', type: 'feature', value: 'auto', + description: 'AVX512BW optimizations') option('keyring', type: 'feature', value: 'auto', description: 'Linux keyring support') diff --git a/migration/ram.c b/migration/ram.c index dc1de9ddbc..ff4c15c9c3 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -83,6 +83,34 @@ /* 0x80 is reserved in migration.h start with 0x100 next */ #define RAM_SAVE_FLAG_COMPRESS_PAGE0x100 +int (*xbzrle_encode_buffer_func)(uint8_t *, uint8_t *, int, + uint8_t *, int) = xbzrle_encode_buffer; #if +defined(CONFIG_AVX512BW_OPT) #include "qemu/cpuid.h" +static void __attribute__((constructor)) init_cpu_flag(void) { +unsigned max = __get_cpuid_max(0, NULL); +int a, b, c, d; +if (max >= 1) { +__cpuid(1, a, b, c, d); + /* We must check that AVX is not just available, but usable. */ +if ((c & bit_OSXSAVE) && (c & bit_AVX) && max >= 7) { +int bv; +__asm("xgetbv" : "=a"(bv), "=d"(d) : "c"(0)); +__cpuid_count(7, 0, a, b, c, d); + /* 0xe6: +* XCR0[7:5] = 111b (OPMASK state, upper 256-bit of ZMM0-ZMM15 +*and ZMM16-ZMM31 state are enabled by OS) +* XCR0[2:1] = 11b (XMM state and YMM state are enabled by OS) +*/ +if ((bv & 0xe6) == 0xe6 && (b & bit_AVX512BW)) { +xbzrle_encode_buffer_func = xbzrle_encode_buffer_avx512; +} +} +} +} +#endif + XBZRLECacheStats xbzrle_counters; /* struct contains XBZRLE cache and a static page @@ -802,9 +830,9 @@ static int save_xbzrle_page(RAMState *rs, uint8_t **current_data, memcpy(XBZRLE.current_buf, *current_data, TARGET_PAGE_SIZE); /* XBZRLE encoding (if there is no overflow) */ -encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf, - TARGET_PAGE_SIZE, XBZRLE.encoded_buf, - TARGET_PAGE_SIZE); +encoded_len = xbzrle_encode_buffer_func(prev_cached_page, XBZRLE.current_buf, +TARGET_PAGE_SIZE, XBZRLE.encoded_buf, +TARGET_PAGE_SIZE); /* * Update the cache contents, so that it corresponds
Re: [PATCH] qboot: update to latest submodule
FYI, that commit made it to: https://github.com/bonzini/qboot But wasn't pushed to: https://github.com/qemu/qboot https://gitlab.com/qemu-project/qboot https://git.qemu.org/?p=qboot.git;a=summary I have no idea what's canonical, except that the submodule in the qemu checkout seems to point to the gitlab instance.