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 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] 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 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 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
>
>
[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 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 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 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 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 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 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 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 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 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: [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
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 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] 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 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
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 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: [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
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,
[PATCH] vhost-user-blk: fix the resize crash
If the os is not installed and doesn't have the virtio guest driver,
the vhost dev isn't started, so the dev->vdev is NULL.
Reproduce: mount a Win 2019 iso, go into the install ui, then resize
the virtio-blk device, qemu crash.
Signed-off-by: Li Feng
---
hw/block/vhost-user-blk.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9117222456..db30bb754f 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -95,6 +95,10 @@ static int vhost_user_blk_handle_config_change(struct
vhost_dev *dev)
VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
Error *local_err = NULL;
+if (!dev->started) {
+return 0;
+}
+
ret = vhost_dev_get_config(dev, (uint8_t *),
sizeof(struct virtio_blk_config),
_err);
--
2.37.3
Re: [PATCH v9 1/7] include: add zoned device structs
Damien Le Moal 于2022年9月19日 周一16:04写道：
> On 9/19/22 09:50, Sam Li wrote:
> > Stefan Hajnoczi 于2022年9月18日周日 04:17写道：
> >>
> >> On Thu, Sep 15, 2022 at 06:06:38PM +0800, Sam Li wrote:
> >>> Eric Blake 于2022年9月15日周四 16:05写道：
>
> On Sat, Sep 10, 2022 at 01:27:53PM +0800, Sam Li wrote:
> > Signed-off-by: Sam Li
> > Reviewed-by: Stefan Hajnoczi
> > Reviewed-by: Damien Le Moal
> > ---
> > include/block/block-common.h | 43
>
> > 1 file changed, 43 insertions(+)
> >
> > diff --git a/include/block/block-common.h
> b/include/block/block-common.h
> > index fdb7306e78..36bd0e480e 100644
> > --- a/include/block/block-common.h
> > +++ b/include/block/block-common.h
> > @@ -49,6 +49,49 @@ typedef struct BlockDriver BlockDriver;
> > typedef struct BdrvChild BdrvChild;
> > typedef struct BdrvChildClass BdrvChildClass;
> >
> > +typedef enum BlockZoneOp {
> > +BLK_ZO_OPEN,
> > +BLK_ZO_CLOSE,
> > +BLK_ZO_FINISH,
> > +BLK_ZO_RESET,
> > +} BlockZoneOp;
> > +
> > +typedef enum BlockZoneModel {
> > +BLK_Z_NONE = 0x0, /* Regular block device */
> > +BLK_Z_HM = 0x1, /* Host-managed zoned block device */
> > +BLK_Z_HA = 0x2, /* Host-aware zoned block device */
> > +} BlockZoneModel;
> > +
> > +typedef enum BlockZoneCondition {
> > +BLK_ZS_NOT_WP = 0x0,
> > +BLK_ZS_EMPTY = 0x1,
> > +BLK_ZS_IOPEN = 0x2,
> > +BLK_ZS_EOPEN = 0x3,
> > +BLK_ZS_CLOSED = 0x4,
> > +BLK_ZS_RDONLY = 0xD,
> > +BLK_ZS_FULL = 0xE,
> > +BLK_ZS_OFFLINE = 0xF,
> > +} BlockZoneCondition;
> > +
> > +typedef enum BlockZoneType {
> > +BLK_ZT_CONV = 0x1, /* Conventional random writes supported */
> > +BLK_ZT_SWR = 0x2, /* Sequential writes required */
> > +BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
> > +} BlockZoneType;
> > +
> > +/*
> > + * Zone descriptor data structure.
> > + * Provides information on a zone with all position and size values
> in bytes.
>
> I'm glad that you chose bytes here for use in qemu. But since the
> kernel struct blk_zone uses sectors instead of bytes, is it worth
> adding a sentence that we intentionally use bytes here, different from
> Linux, to make it easier for reviewers to realize that scaling when
> translating between qemu and kernel is necessary?
> >>>
> >>> Sorry about the unit mistake. The zone information is in sectors which
> >>> is the same as kernel struct blk_zone. I think adding a sentence to
> >>> inform the sector unit makes it clear what the zone descriptor is.
> >>
> >> I'd make the units bytes for consistency with the rest of the QEMU block
> >> layer. For example, the MapEntry structure that "qemu-img map" reports
> >> has names with similar fields and they are in bytes:
> >>
> >> struct MapEntry {
> >> int64_t start;
> >> int64_t length;
> >>
> >
> > I think the zone descriptor uses sector units because ioctl() will
> > report zones in sector units. Making blk_zone.offset =
> > zone_descriptor.offset is more convenient than using byte units where
> > it needs make conversions twice(sector -> byte -> sector in zone
> > descriptors and offset argument in bdrv_co_zone_report). The MapEntry
> > uses byte units because lseek() in bdrv_co_block_status suggests the
> > file offset is set to bytes and I think it may be why the rest of the
> > block layer uses bytes(not sure).
> >
> > I do not object to using bytes here but it would require some
> > compromises. If I was wrong about anything, please let me know.
>
> The conversion can be done using 9-bits left and right shifts, which are
> cheap to do. I think it is important to be consistent with qemu block API,
> so using for the API bytes is preferred. That will avoid confusions.
Ok, will change it. Thanks!
>
> >
> >
> > Sam
>
> --
> Damien Le Moal
> Western Digital Research
>
>
Re: [PATCH v9 1/7] include: add zoned device structs
On 9/19/22 09:50, Sam Li wrote:
> Stefan Hajnoczi 于2022年9月18日周日 04:17写道：
>>
>> On Thu, Sep 15, 2022 at 06:06:38PM +0800, Sam Li wrote:
>>> Eric Blake 于2022年9月15日周四 16:05写道：
On Sat, Sep 10, 2022 at 01:27:53PM +0800, Sam Li wrote:
> Signed-off-by: Sam Li
> Reviewed-by: Stefan Hajnoczi
> Reviewed-by: Damien Le Moal
> ---
> include/block/block-common.h | 43
> 1 file changed, 43 insertions(+)
>
> diff --git a/include/block/block-common.h b/include/block/block-common.h
> index fdb7306e78..36bd0e480e 100644
> --- a/include/block/block-common.h
> +++ b/include/block/block-common.h
> @@ -49,6 +49,49 @@ typedef struct BlockDriver BlockDriver;
> typedef struct BdrvChild BdrvChild;
> typedef struct BdrvChildClass BdrvChildClass;
>
> +typedef enum BlockZoneOp {
> +BLK_ZO_OPEN,
> +BLK_ZO_CLOSE,
> +BLK_ZO_FINISH,
> +BLK_ZO_RESET,
> +} BlockZoneOp;
> +
> +typedef enum BlockZoneModel {
> +BLK_Z_NONE = 0x0, /* Regular block device */
> +BLK_Z_HM = 0x1, /* Host-managed zoned block device */
> +BLK_Z_HA = 0x2, /* Host-aware zoned block device */
> +} BlockZoneModel;
> +
> +typedef enum BlockZoneCondition {
> +BLK_ZS_NOT_WP = 0x0,
> +BLK_ZS_EMPTY = 0x1,
> +BLK_ZS_IOPEN = 0x2,
> +BLK_ZS_EOPEN = 0x3,
> +BLK_ZS_CLOSED = 0x4,
> +BLK_ZS_RDONLY = 0xD,
> +BLK_ZS_FULL = 0xE,
> +BLK_ZS_OFFLINE = 0xF,
> +} BlockZoneCondition;
> +
> +typedef enum BlockZoneType {
> +BLK_ZT_CONV = 0x1, /* Conventional random writes supported */
> +BLK_ZT_SWR = 0x2, /* Sequential writes required */
> +BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
> +} BlockZoneType;
> +
> +/*
> + * Zone descriptor data structure.
> + * Provides information on a zone with all position and size values in
> bytes.
I'm glad that you chose bytes here for use in qemu. But since the
kernel struct blk_zone uses sectors instead of bytes, is it worth
adding a sentence that we intentionally use bytes here, different from
Linux, to make it easier for reviewers to realize that scaling when
translating between qemu and kernel is necessary?
>>>
>>> Sorry about the unit mistake. The zone information is in sectors which
>>> is the same as kernel struct blk_zone. I think adding a sentence to
>>> inform the sector unit makes it clear what the zone descriptor is.
>>
>> I'd make the units bytes for consistency with the rest of the QEMU block
>> layer. For example, the MapEntry structure that "qemu-img map" reports
>> has names with similar fields and they are in bytes:
>>
>> struct MapEntry {
>> int64_t start;
>> int64_t length;
>>
>
> I think the zone descriptor uses sector units because ioctl() will
> report zones in sector units. Making blk_zone.offset =
> zone_descriptor.offset is more convenient than using byte units where
> it needs make conversions twice(sector -> byte -> sector in zone
> descriptors and offset argument in bdrv_co_zone_report). The MapEntry
> uses byte units because lseek() in bdrv_co_block_status suggests the
> file offset is set to bytes and I think it may be why the rest of the
> block layer uses bytes(not sure).
>
> I do not object to using bytes here but it would require some
> compromises. If I was wrong about anything, please let me know.
The conversion can be done using 9-bits left and right shifts, which are
cheap to do. I think it is important to be consistent with qemu block API,
so using for the API bytes is preferred. That will avoid confusions.
>
>
> Sam
--
Damien Le Moal
Western Digital Research
