[PULL 09/16] hw/i386/microvm: Simplify using object_dynamic_cast()
From: Philippe Mathieu-Daudé
Use object_dynamic_cast() to determine if 'dev' is a TYPE_VIRTIO_MMIO.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Sergio Lopez
Signed-off-by: Michael Tokarev
---
hw/i386/microvm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 3d606a20b4..7227a2156c 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -389,9 +389,8 @@ static void microvm_fix_kernel_cmdline(MachineState
*machine)
bus = sysbus_get_default();
QTAILQ_FOREACH(kid, &bus->children, sibling) {
DeviceState *dev = kid->child;
-ObjectClass *class = object_get_class(OBJECT(dev));
-if (class == object_class_by_name(TYPE_VIRTIO_MMIO)) {
+if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO)) {
VirtIOMMIOProxy *mmio = VIRTIO_MMIO(OBJECT(dev));
VirtioBusState *mmio_virtio_bus = &mmio->bus;
BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
--
2.39.2
[PULL 04/16] hw/remote: Fix vfu_cfg trace offset format
From: Mattias Nissler The printed offset value is prefixed with 0x, but was actually printed in decimal. To spare others the confusion, adjust the format specifier to hexadecimal. Signed-off-by: Mattias Nissler Reviewed-by: Jagannathan Raman Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Michael Tokarev --- hw/remote/trace-events | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/remote/trace-events b/hw/remote/trace-events index c167b3c7a5..0d1b7d56a5 100644 --- a/hw/remote/trace-events +++ b/hw/remote/trace-events @@ -5,8 +5,8 @@ mpqemu_recv_io_error(int cmd, int size, int nfds) "failed to receive %d size %d, # vfio-user-obj.c vfu_prop(const char *prop, const char *val) "vfu: setting %s as %s" -vfu_cfg_read(uint32_t offset, uint32_t val) "vfu: cfg: 0x%u -> 0x%x" -vfu_cfg_write(uint32_t offset, uint32_t val) "vfu: cfg: 0x%u <- 0x%x" +vfu_cfg_read(uint32_t offset, uint32_t val) "vfu: cfg: 0x%x -> 0x%x" +vfu_cfg_write(uint32_t offset, uint32_t val) "vfu: cfg: 0x%x <- 0x%x" vfu_dma_register(uint64_t gpa, size_t len) "vfu: registering GPA 0x%"PRIx64", %zu bytes" vfu_dma_unregister(uint64_t gpa) "vfu: unregistering GPA 0x%"PRIx64"" vfu_bar_register(int i, uint64_t addr, uint64_t size) "vfu: BAR %d: addr 0x%"PRIx64" size 0x%"PRIx64"" -- 2.39.2
[PULL 00/16] trivial patches 2023-06-10
Hi here!
This is my first pullreq in quite some years.
It looks like there has been quite some trivial patches which were forgotten,
so I thought I'd give it a shot at least.
And since I haven't done a PR in a while, I basically forgot how to do it
properly :)
Please note: there are 2 patches in there which are *not* reviewed. Being
a trivial-patch collection, they're "obviously correct", - namely these are
spelling fixes ("information") and adding comments in linux-user/syscall.c.
If that's not okay, let's remove these 2 changes and apply the rest.
Thanks,
/mjt
The following changes since commit 5f9dd6a8ce3961db4ce47411ed2097ad88bdf5fc:
Merge tag 'pull-9p-20230608' of https://github.com/cschoenebeck/qemu into
staging (2023-06-08 08:47:35 -0700)
are available in the Git repository at:
https://gitlab.com/mjt0k/qemu.git tags/trivial-patches-20230610
for you to fetch changes up to e928907105cfeb48b68cedce232bbd4784536707:
linux-user: elfload: Specify -R is an option for qemu-user binaries
(2023-06-10 09:40:46 +0300)
trivial-patches-20230610
Anastasia Belova (1):
vnc: move assert in vnc_worker_thread_loop
Andrew Jeffery (2):
linux-user: elfload: s/min_mmap_addr/mmap_min_addr/
linux-user: elfload: Specify -R is an option for qemu-user binaries
Carlos Santos (1):
meson: install keyboard maps only if necessary
Mattias Nissler (1):
hw/remote: Fix vfu_cfg trace offset format
Michael Tokarev (4):
hw/virtio/virtio-qmp.c: spelling: suppoted
spelling: information
block.c: add newline for "Detected format" warning
linux-user: add comments for TARGET_NR_[gs]etgroups{,32}
Milan Zamazal (1):
docs: Fix trivial typos in vhost-user.rst
Peter Maydell (1):
linux-user: Return EINVAL for getgroups() with negative gidsetsize
Philippe Mathieu-Daudé (5):
target/m68k/fpu_helper: Use FloatRelation enum to hold comparison result
hw/core/cpu: Simplify realize() using MACHINE_GET_CLASS() macro
hw/i386/microvm: Simplify using object_dynamic_cast()
hw/pci/pci: Simplify pci_bar_address() using MACHINE_GET_CLASS() macro
hw/usb/hcd-ehci-pci: Simplify using DEVICE_GET_CLASS() macro
block.c| 2 +-
docs/system/devices/vhost-user.rst | 4 ++--
hw/core/cpu-common.c | 3 +--
hw/i386/microvm.c | 3 +--
hw/pci/pci.c | 4 +---
hw/remote/trace-events | 4 ++--
hw/usb/hcd-ehci-pci.c | 2 +-
hw/virtio/virtio-qmp.c | 2 +-
include/ui/clipboard.h | 2 +-
linux-user/elfload.c | 5 +++--
linux-user/syscall.c | 12 ++--
pc-bios/keymaps/meson.build| 6 --
qapi/cryptodev.json| 2 +-
qga/qapi-schema.json | 2 +-
target/m68k/fpu_helper.c | 4 ++--
ui/vnc-jobs.c | 3 ++-
16 files changed, 30 insertions(+), 30 deletions(-)
[PULL 02/16] hw/virtio/virtio-qmp.c: spelling: suppoted
Fixes: f3034ad71fcd0a6a58bc37830f182b307f089159
Signed-off-by: Michael Tokarev
Reviewed-by: Stefan Weil
---
hw/virtio/virtio-qmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index b5e1835299..3528fc628d 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -331,7 +331,7 @@ static const qmp_virtio_feature_map_t
virtio_net_feature_map[] = {
static const qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
FEATURE_ENTRY(VIRTIO_SCSI_F_INOUT, \
"VIRTIO_SCSI_F_INOUT: Requests including read and writable data "
-"buffers suppoted"),
+"buffers supported"),
FEATURE_ENTRY(VIRTIO_SCSI_F_HOTPLUG, \
"VIRTIO_SCSI_F_HOTPLUG: Reporting and handling hot-plug events "
"supported"),
--
2.39.2
[PULL 15/16] linux-user: elfload: s/min_mmap_addr/mmap_min_addr/
From: Andrew Jeffery
As-is the error message can cause some confusion as the mentioned sysctl
attribute name is wrong:
https://www.kernel.org/doc/html/latest/admin-guide/sysctl/vm.html#mmap-min-addr
Signed-off-by: Andrew Jeffery
Reviewed-by: Michael Tokarev
Signed-off-by: Michael Tokarev
---
linux-user/elfload.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index d80d68484b..76874833e3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2798,7 +2798,7 @@ static void pgb_reserved_va(const char *image_name,
abi_ulong guest_loaddr,
if (addr == MAP_FAILED || addr != test) {
error_report("Unable to reserve 0x%lx bytes of virtual address "
"space at %p (%s) for use as guest address space (check
your "
- "virtual memory ulimit setting, min_mmap_addr or reserve
less "
+ "virtual memory ulimit setting, mmap_min_addr or reserve
less "
"using -R option)", reserved_va + 1, test,
strerror(errno));
exit(EXIT_FAILURE);
}
--
2.39.2
Re: [PATCH] accel/kvm/kvm-all: Handle register access errors
On 2022/12/01 20:00, Akihiko Odaki wrote: On 2022/12/01 19:40, Peter Maydell wrote: On Thu, 1 Dec 2022 at 10:27, Akihiko Odaki wrote: A register access error typically means something seriously wrong happened so that anything bad can happen after that and recovery is impossible. Even failing one register access is catastorophic as architecture-specific code are not written so that it torelates such failures. Make sure the VM stop and nothing worse happens if such an error occurs. Signed-off-by: Akihiko Odaki In a similar vein there was also https://lore.kernel.org/all/20220617144857.34189-1-pet...@redhat.com/ back in June, which on the one hand was less comprehensive but on the other does the plumbing to pass the error upwards rather than reporting it immediately at point of failure. I'm in principle in favour but suspect we'll run into some corner cases where we were happily ignoring not-very-important failures (eg if you're running Linux as the host OS on a Mac M1 and your host kernel doesn't have this fix: https://lore.kernel.org/all/ynhz6cw5onr2e...@google.com/T/ then QEMU will go from "works by sheer luck" to "consistently hits this error check"). So we should aim to land this extra error checking early in the release cycle so we have plenty of time to deal with any bug reports we get about it. thanks -- PMM Actually I found this problem when I tried to run QEMU with KVM on M2 MacBook Air and encountered a failure described and fixed at: https://lore.kernel.org/all/20221201104914.28944-2-akihiko.od...@daynix.com/ Although the affected register was not really important, QEMU couldn't run the guest well enough because kvm_arch_put_registers for ARM64 is written in a way that it fails early. I guess the situation is not so different for other architectures as well. I still agree that this should be postponed until a new release cycle starts as register saving/restoring is too important to fail. Regards, Akihiko Odaki Hi, QEMU 8.0 is already released so I think it's time to revisit this. Regards, Akihiko Odaki
Re: [PATCH] accel/kvm: Specify default IPA size for arm64
On 2023/04/24 19:58, Akihiko Odaki wrote:
On 2023/01/16 20:18, Peter Maydell wrote:
On Sat, 14 Jan 2023 at 06:49, Akihiko Odaki
wrote:
On 2023/01/14 14:23, Richard Henderson wrote:
On 1/8/23 22:22, Akihiko Odaki wrote:
libvirt uses "none" machine type to test KVM availability. Before this
change, QEMU used to pass 0 as machine type when calling
KVM_CREATE_VM.
The kernel documentation says:
On arm64, the physical address size for a VM (IPA Size limit) is
limited to 40bits by default. The limit can be configured if the host
supports the extension KVM_CAP_ARM_VM_IPA_SIZE. When supported, use
KVM_VM_TYPE_ARM_IPA_SIZE(IPA_Bits) to set the size in the machine
type
identifier, where IPA_Bits is the maximum width of any physical
address used by the VM. The IPA_Bits is encoded in bits[7-0] of the
machine type identifier.
e.g, to configure a guest to use 48bit physical address size::
vm_fd = ioctl(dev_fd, KVM_CREATE_VM,
KVM_VM_TYPE_ARM_IPA_SIZE(48));
The requested size (IPA_Bits) must be:
== =
0 Implies default size, 40bits (for backward compatibility)
N Implies N bits, where N is a positive integer such that,
32 <= N <= Host_IPA_Limit
== =
Host_IPA_Limit is the maximum possible value for IPA_Bits on the host
and is dependent on the CPU capability and the kernel configuration.
The limit can be retrieved using KVM_CAP_ARM_VM_IPA_SIZE of the
KVM_CHECK_EXTENSION ioctl() at run-time.
Creation of the VM will fail if the requested IPA size (whether it is
implicit or explicit) is unsupported on the host.
https://docs.kernel.org/virt/kvm/api.html#kvm-create-vm
So if Host_IPA_Limit < 40, such KVM_CREATE_VM will fail, and libvirt
incorrectly thinks KVM is not available. This actually happened on M2
MacBook Air.
Fix this by specifying 32 for IPA_Bits as any arm64 system should
support the value according to the documentation.
Signed-off-by: Akihiko Odaki
---
accel/kvm/kvm-all.c | 4
1 file changed, 4 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index e86c33e0e6..776ac7efcc 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2294,7 +2294,11 @@ static int kvm_init(MachineState *ms)
KVMState *s;
const KVMCapabilityInfo *missing_cap;
int ret;
+#ifdef TARGET_AARCH64
+ int type = 32;
+#else
int type = 0;
+#endif
No need for an ifdef. Down below we have,
if (object_property_find(OBJECT(current_machine), "kvm-type")) {
g_autofree char *kvm_type =
object_property_get_str(OBJECT(current_machine),
"kvm-type",
&error_abort);
type = mc->kvm_type(ms, kvm_type);
} else if (mc->kvm_type) {
type = mc->kvm_type(ms, NULL);
}
and the aarch64 -M virt machine provides virt_kvm_type as mc->kvm_type.
How did you hit this? Are you trying to implement your own board
model?
Looking at this, I'm surprised this is a board hook and not a cpu hook.
But I suppose the architecture specific 'type' can hide any number of
sins. Anyway, if you are doing your own board model, I suggest
arranging to share the virt board hook -- maybe moving it to
target/arm/kvm.c in the process?
I hit this problem when I used libvirt; libvirt uses "none" machine type
to probe the availability of KVM and "none" machine type does not
provide kvm_type hook.
As the implementation of "none" machine type is shared among different
architectures, we cannot remove ifdef by moving it to the hook.
Although implementing the hook for "none" machine type is still
possible, I think the default type should provide the lowest common
denominator and "none" machine type shouldn't try to work around when
the type is wrong. Otherwise it doesn't make sense to provide the
"default".
Yes, the problem is that the 'none' board type is all
architecture-independent code, and so is this kvm_init() code, so
there's no obvious arm-specific place to say "pick the best IPA size
that will work for this host".
Perhaps we should create somewhere in here a target-arch specific
hook: we already have ifdefs in this function for S390X and PPC
(printing some special case error strings if the ioctl fails), so
maybe a hook that does "take the type provided by the machine hook,
if any, sanitize or reject it, do the ioctl call, print arch-specific
help/error messages if relevant" ? Paolo, do you have an opinion?
thanks
-- PMM
Hi Paolo,
I have sent this patch a while ago but it's kind of missed so I'm about
to push this forward again. Can you have a look at this?
Regards,
Akihiko Odaki
ping?
Re: QEMU virt (arm64) does not honor reserved-memory set in device tree
Hi Mohd,
On 6/10/23 10:01 AM, Mohd Yusuf Abdul Hamid wrote:
I am trying to reserve a portion of the system memory in QEMU (arm64 virt),
v7.2.1 - but the kernel never honors the reserved memory area and keeps using
the area.
Say, I dumped out DTB and added:
reserved-memory {
#address-cells = <0x02>;
#size-cells = <0x02>;
rsvdram@5000 {
no-map;
reg = <0x00 0x5000 0x00 0x2000>;
};
};
When booted, /proc/iomem still shows the kernel is using the entire space - eg
2GB.
Is this a supported feature or I would need to modify the virt.c and define
scratch area for some device driver scratch area.
It relies on the guest kernel to handle the device-tree and the device-tree node
for the reserved map. I doubt if you had ACPI over device-tree in the guest
kernel's
configuration. In this case, the reserved memory regions need to be specified in
ACPI tables instead of device-tree.
Thanks,
Gavin
QEMU virt (arm64) does not honor reserved-memory set in device tree
Hi,
I am trying to reserve a portion of the system memory in QEMU (arm64 virt),
v7.2.1 - but the kernel never honors the reserved memory area and keeps
using the area.
Say, I dumped out DTB and added:
reserved-memory {
#address-cells = <0x02>;
#size-cells = <0x02>;
rsvdram@5000 {
no-map;
reg = <0x00 0x5000 0x00 0x2000>;
};
};
When booted, /proc/iomem still shows the kernel is using the entire space -
eg 2GB.
Is this a supported feature or I would need to modify the virt.c and define
scratch area for some device driver scratch area.
Mohd Yusuf Abdul Hamid
Re: [EXT] Re: [Qemu RFC 0/7] Early enabling of DCD emulation in Qemu
From: Ira Weiny
Sent: Friday, June 9, 2023 2:06 PM
To: ni...@outlook.com ; Ira Weiny ;
Shesha Bhushan Sreenivasamurthy
Cc: Shesha Bhushan Sreenivasamurthy ; Fan Ni
; Jonathan Cameron ;
qemu-devel@nongnu.org ; linux-...@vger.kernel.org
; gregory.pr...@memverge.com
; hch...@avery-design.com.tw
; cbr...@avery-design.com
; dan.j.willi...@intel.com ;
Adam Manzanares ; d...@stgolabs.net
; nmtadam.sams...@gmail.com
Subject: [EXT] Re: [Qemu RFC 0/7] Early enabling of DCD emulation in Qemu
External Email
--
ni...@outlook.com wrote:
> The 06/08/2023 08:43, Ira Weiny wrote:
> > Shesha Bhushan Sreenivasamurthy wrote:
[snip]
>
> Hi Ira & Shesha,
> FYI. I reabased my patch series on top of the above branch and created a new
> branch here:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_moking_qemu-2Ddcd-2Dpreview-2Dlatest_tree_dcd-2Dpreview&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Zta64bwn4nurTRpD4LY2OGr8KklkMRPn7Z_Qy0o4unU&m=SvyB_49EIFUT8-ZEVgIEYYjU6-zGTX4wb30kuNLUhkTSHYZK5-C0Gxr7uvefhtj4&s=MFD7qlSaTuy-w6aDmavIMbSP_aeaqZmSML7IVOX5jLs&e=
>
Thanks!
>
> It passes the same tests as shown here:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_linux-2Dcxl_6481f70fca5c2-5Fc82be29440-40iweiny-2Dmobl.notmuch_T_-23m76f6e85ce3d7292b1982960eb22086ee03922166&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Zta64bwn4nurTRpD4LY2OGr8KklkMRPn7Z_Qy0o4unU&m=SvyB_49EIFUT8-ZEVgIEYYjU6-zGTX4wb30kuNLUhkTSHYZK5-C0Gxr7uvefhtj4&s=e-fQOi0RzSZXxfSz37Bpz1sKtp7Yy0MWqonZnswK0RU&e=
>
I've not gotten very far with this testing. But I did find that regular
type 3 devices don't work with this change. I used the patch below to get
this working. Was there something I was missing to configure a non-DCD
device?
I don't particularly like adding another bool to this call stack. Seems
like this calls for a flags field but I want to move on to DCD work so I
hacked this in.
I am working on the DCD FM-API commands here -
https://gitlab.com/sheshas/qemu-fmapi/-/tree/cxl-2023-05-25
-Shesha
Ira
commit ed27935044dcbd2c6ba71f8411b218621f3f4167
Author: Ira Weiny
Date: Fri Jun 9 13:56:33 2023 -0700
hw/mem/cxl_type3: Exclude DCD from CEL when type3 is not DCD
Per CXL 3.0 9.13.3 Dynamic Capacity Device (DCD) when the type 3 memory
device does not have DCD support the CEL should not include DCD
configuration commands.
If the number of DC regions supported is 0 skip the DCD commands in the
CEL.
Applies on top of Fan Ni's work here:
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_moking_qemu-2Ddcd-2Dpreview-2Dlatest_tree_dcd-2Dpreview&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Zta64bwn4nurTRpD4LY2OGr8KklkMRPn7Z_Qy0o4unU&m=SvyB_49EIFUT8-ZEVgIEYYjU6-zGTX4wb30kuNLUhkTSHYZK5-C0Gxr7uvefhtj4&s=MFD7qlSaTuy-w6aDmavIMbSP_aeaqZmSML7IVOX5jLs&e=
Not-yet-Signed-off-by: Ira Weiny
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index a4a2c6a80004..262e35935563 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -288,7 +288,7 @@ static void mailbox_reg_init_common(CXLDeviceState
*cxl_dstate)
static void memdev_reg_init_common(CXLDeviceState *cxl_dstate) { }
-void cxl_device_register_init_common(CXLDeviceState *cxl_dstate)
+void cxl_device_register_init_common(CXLDeviceState *cxl_dstate, bool is_dcd)
{
uint64_t *cap_hdrs = cxl_dstate->caps_reg_state64;
const int cap_count = 3;
@@ -307,7 +307,7 @@ void cxl_device_register_init_common(CXLDeviceState
*cxl_dstate)
cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000, 1);
memdev_reg_init_common(cxl_dstate);
- cxl_initialize_mailbox(cxl_dstate, false);
+ cxl_initialize_mailbox(cxl_dstate, false, is_dcd);
}
void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate)
@@ -329,7 +329,7 @@ void cxl_device_register_init_swcci(CXLDeviceState
*cxl_dstate)
cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000, 1);
memdev_reg_init_common(cxl_dstate);
- cxl_initialize_mailbox(cxl_dstate, true);
+ cxl_initialize_mailbox(cxl_dstate, true, false);
}
uint64_t cxl_device_get_timestamp(CXLDeviceState *cxl_dstate)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 93b26e717c94..80e9cb9a8f04 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1526,7 +1526,8 @@ static void bg_timercb(void *opaque)
}
}
-void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
+void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci,
+ bool is_dcd)
{
if (!switch_cci) {
cxl_dstate->cxl_cmd_set = cxl_cmd_set;
@@ -1534,6 +1535,9 @@ void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate,
bool switch_cci)
cxl_dstate->cxl_cmd_set = cxl_cmd_set_sw;
}
for (int set = 0; set < 256; set++) {
+ if (!is_dcd && set == DCD_CONFIG) {
+
Re: [PULL v4 09/10] hw/arm: introduce xenpvh machine
On 6/9/23 10:07, Stefano Stabellini wrote: From: Vikram Garhwal Add a new machine xenpvh which creates a IOREQ server to register/connect with Xen Hypervisor. Optional: When CONFIG_TPM is enabled, it also creates a tpm-tis-device, adds a TPM emulator and connects to swtpm running on host machine via chardev socket and support TPM functionalities for a guest domain. Extra command line for aarch64 xenpvh QEMU to connect to swtpm: -chardev socket,id=chrtpm,path=/tmp/myvtpm2/swtpm-sock \ -tpmdev emulator,id=tpm0,chardev=chrtpm \ -machine tpm-base-addr=0x0c00 \ swtpm implements a TPM software emulator(TPM 1.2 & TPM 2) built on libtpms and provides access to TPM functionality over socket, chardev and CUSE interface. Github repo:https://github.com/stefanberger/swtpm Example for starting swtpm on host machine: mkdir /tmp/vtpm2 swtpm socket --tpmstate dir=/tmp/vtpm2 \ --ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock & Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Reviewed-by: Stefano Stabellini --- docs/system/arm/xenpvh.rst| 34 +++ docs/system/target-arm.rst| 1 + hw/arm/meson.build| 2 + hw/arm/xen_arm.c | 181 ++ include/hw/arm/xen_arch_hvm.h | 9 ++ include/hw/xen/arch_hvm.h | 2 + 6 files changed, 229 insertions(+) create mode 100644 docs/system/arm/xenpvh.rst create mode 100644 hw/arm/xen_arm.c create mode 100644 include/hw/arm/xen_arch_hvm.h Fails testing. Summary of Failures: 10/423 qemu:qtest+qtest-aarch64 / qtest-aarch64/test-hmp ERROR 161.45s killed by signal 6 SIGABRT 210/423 qemu:qtest+qtest-aarch64 / qtest-aarch64/qom-test ERROR 327.56s killed by signal 6 SIGABRT # starting QEMU: exec ./qemu-system-aarch64 -qtest unix:/tmp/qtest-669817.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-669817.qmp,id=char0 -mon chardev=char0,mode=control -display none -machine xenpvh -accel qtest qemu-system-aarch64: The -accel and "-machine accel=" options are incompatible socket_accept failed: Resource temporarily unavailable ** ERROR:../src/tests/qtest/libqtest.c:474:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0) Bail out! ERROR:../src/tests/qtest/libqtest.c:474:qtest_init_without_qmp_handshake: assertion failed: (s->fd >= 0 && s->qmp_fd >= 0) ../src/tests/qtest/libqtest.c:186: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) r~
Re: [PATCH] hw/i386/pc: Clean up pc_machine_initfn
On 9/6/23 18:41, Suravee Suthikulpanit wrote: To use the newly introduced PC machine class local variable. Suggedted-by: Igor Mammedov "Suggested-by" Signed-off-by: Suravee Suthikulpanit --- hw/i386/pc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 33ffb03a32..f8d105e829 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1864,7 +1864,7 @@ static void pc_machine_initfn(Object *obj) pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type; /* acpi build is enabled by default if machine supports it */ -pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build; +pcms->acpi_build_enabled = pcmc->has_acpi_build; pcms->smbus_enabled = true; pcms->sata_enabled = true; pcms->i8042_enabled = true;
Re: [PATCH v3 5/5] intel_iommu: Optimize out some unnecessary UNMAP calls
On Fri, Jun 09, 2023 at 05:49:06AM +, Duan, Zhenzhong wrote: > Seems vtd_page_walk_one() already works in above way, checking mapping > changes and calling kernel for changed entries? Agreed in most cases, but the path this patch modified is not? E.g. it happens in rare cases where we simply want to unmap everything (e.g. on a system reset, or invalid context entry)? That's also why I'm curious whether perf of this path matters at all (and assuming now we all agree that's the only goal now..), because afaiu it didn't really trigger in common paths. -- Peter Xu
Re: [Qemu RFC 0/7] Early enabling of DCD emulation in Qemu
ni...@outlook.com wrote:
> The 06/08/2023 08:43, Ira Weiny wrote:
> > Shesha Bhushan Sreenivasamurthy wrote:
[snip]
>
> Hi Ira & Shesha,
> FYI. I reabased my patch series on top of the above branch and created a new
> branch here:
>
> https://github.com/moking/qemu-dcd-preview-latest/tree/dcd-preview
Thanks!
>
> It passes the same tests as shown here:
> https://lore.kernel.org/linux-cxl/6481f70fca5c2_c82be29440@iweiny-mobl.notmuch/T/#m76f6e85ce3d7292b1982960eb22086ee03922166
I've not gotten very far with this testing. But I did find that regular
type 3 devices don't work with this change. I used the patch below to get
this working. Was there something I was missing to configure a non-DCD
device?
I don't particularly like adding another bool to this call stack. Seems
like this calls for a flags field but I want to move on to DCD work so I
hacked this in.
Ira
commit ed27935044dcbd2c6ba71f8411b218621f3f4167
Author: Ira Weiny
Date: Fri Jun 9 13:56:33 2023 -0700
hw/mem/cxl_type3: Exclude DCD from CEL when type3 is not DCD
Per CXL 3.0 9.13.3 Dynamic Capacity Device (DCD) when the type 3 memory
device does not have DCD support the CEL should not include DCD
configuration commands.
If the number of DC regions supported is 0 skip the DCD commands in the
CEL.
Applies on top of Fan Ni's work here:
https://github.com/moking/qemu-dcd-preview-latest/tree/dcd-preview
Not-yet-Signed-off-by: Ira Weiny
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index a4a2c6a80004..262e35935563 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -288,7 +288,7 @@ static void mailbox_reg_init_common(CXLDeviceState
*cxl_dstate)
static void memdev_reg_init_common(CXLDeviceState *cxl_dstate) { }
-void cxl_device_register_init_common(CXLDeviceState *cxl_dstate)
+void cxl_device_register_init_common(CXLDeviceState *cxl_dstate, bool is_dcd)
{
uint64_t *cap_hdrs = cxl_dstate->caps_reg_state64;
const int cap_count = 3;
@@ -307,7 +307,7 @@ void cxl_device_register_init_common(CXLDeviceState
*cxl_dstate)
cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000, 1);
memdev_reg_init_common(cxl_dstate);
-cxl_initialize_mailbox(cxl_dstate, false);
+cxl_initialize_mailbox(cxl_dstate, false, is_dcd);
}
void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate)
@@ -329,7 +329,7 @@ void cxl_device_register_init_swcci(CXLDeviceState
*cxl_dstate)
cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000, 1);
memdev_reg_init_common(cxl_dstate);
-cxl_initialize_mailbox(cxl_dstate, true);
+cxl_initialize_mailbox(cxl_dstate, true, false);
}
uint64_t cxl_device_get_timestamp(CXLDeviceState *cxl_dstate)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 93b26e717c94..80e9cb9a8f04 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -1526,7 +1526,8 @@ static void bg_timercb(void *opaque)
}
}
-void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
+void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci,
+bool is_dcd)
{
if (!switch_cci) {
cxl_dstate->cxl_cmd_set = cxl_cmd_set;
@@ -1534,6 +1535,9 @@ void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate,
bool switch_cci)
cxl_dstate->cxl_cmd_set = cxl_cmd_set_sw;
}
for (int set = 0; set < 256; set++) {
+if (!is_dcd && set == DCD_CONFIG) {
+continue;
+}
for (int cmd = 0; cmd < 256; cmd++) {
if (cxl_dstate->cxl_cmd_set[set][cmd].handler) {
struct cxl_cmd *c = &cxl_dstate->cxl_cmd_set[set][cmd];
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 329e8b5915b3..e6e6e125990c 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -1276,9 +1276,11 @@ static void ct3d_reset(DeviceState *dev)
CXLType3Dev *ct3d = CXL_TYPE3(dev);
uint32_t *reg_state = ct3d->cxl_cstate.crb.cache_mem_registers;
uint32_t *write_msk = ct3d->cxl_cstate.crb.cache_mem_regs_write_mask;
+bool is_dcd;
cxl_component_register_init_common(reg_state, write_msk,
CXL2_TYPE3_DEVICE);
-cxl_device_register_init_common(&ct3d->cxl_dstate);
+is_dcd = (ct3d->dc.num_regions != 0);
+cxl_device_register_init_common(&ct3d->cxl_dstate, is_dcd);
}
static Property ct3_props[] = {
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 1ccddcca7d0d..4621bba4f533 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -233,7 +233,7 @@ typedef struct cxl_device_state {
void cxl_device_register_block_init(Object *obj, CXLDeviceState *dev);
/* Set up default values for the register block */
-void cxl_device_register_init_common(CXLDeviceState *dev);
+void cxl_device_register_init_common(CXLDeviceState *dev, bool is_dcd);
void cxl_device_register_init_swcci(CXLDeviceState *dev);
/*
Re: [PATCH v3 2/8] target/arm: v8.3 PAC ID_AA64ISAR[12] feature-detection
On 6/9/23 10:23, Aaron Lindsay wrote:
+static inline int isar_feature_pauth_get_features(const ARMISARegisters *id)
+{
+if (isar_feature_aa64_pauth_arch_qarma5(id)) {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA);
+} else if (isar_feature_aa64_pauth_arch_qarma3(id)) {
+return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3);
+} else {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, API);
+}
+}
As I mentioned in previous review, exactly one of these fields will be non-zero, so you
can just OR them all together without the conditionals.
r~
Re: [PATCH v3 1/8] target/arm: Add ID_AA64ISAR2_EL1
On 6/9/23 10:23, Aaron Lindsay wrote:
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -847,6 +847,7 @@ static bool
hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
{ HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 },
{ HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 },
{ HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 },
+{ HV_SYS_REG_ID_AA64ISAR2_EL1, &host_isar.id_aa64isar2 },
Sadly not defined for MacOSX13.1.sdk, and it's an enum so you can't #ifdef it
either.
You'll need a meson probe for it.
Otherwise, looks good.
r~
Re: [PATCH 0/2] linux-user: Clarify error on failure to map guest address space
27.03.2023 14:55, Andrew Jeffery wrote: Hello, This series is a couple of trivial improvements to the error message from linux-user's ELF loader when it fails to mmap() the guest's address space. Both issues caused me brief confusion when trying to sort myself out after hitting https://gitlab.com/qemu-project/qemu/-/issues/447 I've build tested the two as a sanity check. Ping? That seems like some nice rewording, but not my area.. Thanks! /mjt
Re: [PATCH 21/22] target/arm/tcg: Remove inclusions of 'exec/helper-[proto/gen].h'
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: diff --git a/target/arm/tcg/vec_helper.c b/target/arm/tcg/vec_helper.c index 8fb44a8b7f..5621728271 100644 --- a/target/arm/tcg/vec_helper.c +++ b/target/arm/tcg/vec_helper.c @@ -19,12 +19,16 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/helper-proto.h" #include "tcg/tcg-gvec-desc.h" #include "fpu/softfloat.h" #include "qemu/int128.h" #include "vec_internal.h" +#define HELPER_H "tcg/helper.h.inc" +#include "exec/helper-proto.h.inc" +#include "exec/helper-gen.h.inc" +#undef HELPER_H I am surprised that helper.h.inc would be required here and in most of the other foo_helper.c, where helper-foo.h.inc has already been used. r~
Re: [PATCH] vnc: move assert in vnc_worker_thread_loop
09.06.2023 12:23, Anastasia Belova wrote:
job may be NULL if queue->exit is true. Check
it before dereference job.
Fixes: f31f9c1080 ("vnc: add magic cookie to VncState")
Signed-off-by: Anastasia Belova
---
ui/vnc-jobs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 886f9bf611..fcca7ec632 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -250,12 +250,13 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
/* Here job can only be NULL if queue->exit is true */
job = QTAILQ_FIRST(&queue->jobs);
vnc_unlock_queue(queue);
-assert(job->vs->magic == VNC_MAGIC);
if (queue->exit) {
return -1;
}
+assert(job->vs->magic == VNC_MAGIC);
+
This is a good one, I like it :)
Reviewed-by: Michael Tokarev
It can be applied though -trivial queue.
Thank you!
/mjt
[PATCH v2 07/10] block: Convert qmp_query_named_block_nodes to coroutine
From: Lin Ma
We're converting callers of bdrv_get_allocated_file_size() to run in
coroutines because that function will be made asynchronous when called
(indirectly) from the QMP dispatcher.
This QMP command is a candidate because it indirectly calls
bdrv_get_allocated_file_size() through bdrv_block_device_info() ->
bdrv_query_image_info() -> bdrv_query_image_info().
The previous patches have determined that bdrv_query_image_info() and
bdrv_do_query_node_info() are coroutine-safe so we can just make the
QMP command run in a coroutine.
Signed-off-by: Lin Ma
Signed-off-by: Fabiano Rosas
---
block.c | 2 +-
blockdev.c | 6 +++---
qapi/block-core.json | 3 ++-
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index f94cee8930..abed744b60 100644
--- a/block.c
+++ b/block.c
@@ -6148,7 +6148,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(bool flat,
list = NULL;
QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
-BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, flat, errp);
+BlockDeviceInfo *info = bdrv_co_block_device_info(NULL, bs, flat,
errp);
if (!info) {
qapi_free_BlockDeviceInfoList(list);
return NULL;
diff --git a/blockdev.c b/blockdev.c
index e6eba61484..8b5f7d06c8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2818,9 +2818,9 @@ void qmp_drive_backup(DriveBackup *backup, Error **errp)
blockdev_do_action(&action, errp);
}
-BlockDeviceInfoList *qmp_query_named_block_nodes(bool has_flat,
- bool flat,
- Error **errp)
+BlockDeviceInfoList *coroutine_fn qmp_query_named_block_nodes(bool has_flat,
+ bool flat,
+ Error **errp)
{
bool return_flat = has_flat && flat;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5dd5f7e4b0..9d4c92f2c9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1972,7 +1972,8 @@
{ 'command': 'query-named-block-nodes',
'returns': [ 'BlockDeviceInfo' ],
'data': { '*flat': 'bool' },
- 'allow-preconfig': true }
+ 'allow-preconfig': true,
+ 'coroutine': true}
##
# @XDbgBlockGraphNodeType:
--
2.35.3
[PATCH v2 09/10] block: Convert qmp_query_block() to coroutine_fn
This is another caller of bdrv_get_allocated_file_size() that needs to
be converted to a coroutine because that function will be made
asynchronous when called (indirectly) from the QMP dispatcher.
This QMP command is a candidate because it calls bdrv_do_query_node_info(),
which in turn calls bdrv_get_allocated_file_size().
We've determined bdrv_do_query_node_info() to be coroutine-safe (see
previous commits), so we can just put this QMP command in a coroutine.
Since qmp_query_block() now expects to run in a coroutine, its callers
need to be converted as well. Convert hmp_info_block(), which calls
only coroutine-safe code, including qmp_query_named_block_nodes()
which has been converted to coroutine in the previous patches.
Now that all callers of bdrv_[co_]block_device_info() are using the
coroutine version, a few things happen:
- we can return to using bdrv_block_device_info() without a wrapper;
- bdrv_get_allocated_file_size() can stop being mixed;
- bdrv_co_get_allocated_file_size() needs to be put under the graph
lock because it is being called wthout the wrapper;
- bdrv_do_query_node_info() doesn't need to acquire the AioContext
because it doesn't call aio_poll anymore;
Signed-off-by: Fabiano Rosas
---
block.c| 2 +-
block/monitor/block-hmp-cmds.c | 2 +-
block/qapi.c | 18 +-
hmp-commands-info.hx | 1 +
include/block/block-hmp-cmds.h | 2 +-
include/block/block-io.h | 2 +-
include/block/qapi.h | 12
qapi/block-core.json | 2 +-
8 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/block.c b/block.c
index abed744b60..f94cee8930 100644
--- a/block.c
+++ b/block.c
@@ -6148,7 +6148,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(bool flat,
list = NULL;
QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
-BlockDeviceInfo *info = bdrv_co_block_device_info(NULL, bs, flat,
errp);
+BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, flat, errp);
if (!info) {
qapi_free_BlockDeviceInfoList(list);
return NULL;
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 26116fe831..1049f9b006 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -742,7 +742,7 @@ static void print_block_info(Monitor *mon, BlockInfo *info,
}
}
-void hmp_info_block(Monitor *mon, const QDict *qdict)
+void coroutine_fn hmp_info_block(Monitor *mon, const QDict *qdict)
{
BlockInfoList *block_list, *info;
BlockDeviceInfoList *blockdev_list, *blockdev;
diff --git a/block/qapi.c b/block/qapi.c
index 20660e15d6..3b4bc0b782 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -41,10 +41,10 @@
#include "qemu/qemu-print.h"
#include "sysemu/block-backend.h"
-BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk,
-BlockDriverState *bs,
-bool flat,
-Error **errp)
+BlockDeviceInfo *coroutine_fn bdrv_block_device_info(BlockBackend *blk,
+ BlockDriverState *bs,
+ bool flat,
+ Error **errp)
{
ImageInfo **p_image_info;
ImageInfo *backing_info;
@@ -235,8 +235,6 @@ static void bdrv_do_query_node_info(BlockDriverState *bs,
int ret;
Error *err = NULL;
-aio_context_acquire(bdrv_get_aio_context(bs));
-
size = bdrv_getlength(bs);
if (size < 0) {
error_setg_errno(errp, -size, "Can't get image size '%s'",
@@ -249,7 +247,9 @@ static void bdrv_do_query_node_info(BlockDriverState *bs,
info->filename= g_strdup(bs->filename);
info->format = g_strdup(bdrv_get_format_name(bs));
info->virtual_size= size;
-info->actual_size = bdrv_get_allocated_file_size(bs);
+bdrv_graph_co_rdlock();
+info->actual_size = bdrv_co_get_allocated_file_size(bs);
+bdrv_graph_co_rdunlock();
info->has_actual_size = info->actual_size >= 0;
if (bs->encrypted) {
info->encrypted = true;
@@ -305,7 +305,7 @@ static void bdrv_do_query_node_info(BlockDriverState *bs,
}
out:
-aio_context_release(bdrv_get_aio_context(bs));
+return;
}
/**
@@ -668,7 +668,7 @@ bdrv_query_bds_stats(BlockDriverState *bs, bool blk_level)
return s;
}
-BlockInfoList *qmp_query_block(Error **errp)
+BlockInfoList *coroutine_fn qmp_query_block(Error **errp)
{
BlockInfoList *head = NULL, **p_next = &head;
BlockBackend *blk;
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 47d63d26db..996895f417 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -65,6 +65,7 @@ ERST
.help = "show info of one block device or all bl
[PATCH v2 01/10] block: Remove bdrv_query_block_node_info
The last call site of this function has been removed by commit
c04d0ab026 ("qemu-img: Let info print block graph").
Reviewed-by: Claudio Fontana
Signed-off-by: Fabiano Rosas
---
block/qapi.c | 27 ---
include/block/qapi.h | 3 ---
2 files changed, 30 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index f34f95e0ef..79bf80c503 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -309,33 +309,6 @@ out:
aio_context_release(bdrv_get_aio_context(bs));
}
-/**
- * bdrv_query_block_node_info:
- * @bs: block node to examine
- * @p_info: location to store node information
- * @errp: location to store error information
- *
- * Store image information about @bs in @p_info.
- *
- * @p_info will be set only on success. On error, store error in @errp.
- */
-void bdrv_query_block_node_info(BlockDriverState *bs,
-BlockNodeInfo **p_info,
-Error **errp)
-{
-BlockNodeInfo *info;
-ERRP_GUARD();
-
-info = g_new0(BlockNodeInfo, 1);
-bdrv_do_query_node_info(bs, info, errp);
-if (*errp) {
-qapi_free_BlockNodeInfo(info);
-return;
-}
-
-*p_info = info;
-}
-
/**
* bdrv_query_image_info:
* @bs: block node to examine
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 18d48ddb70..8663971c58 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -36,9 +36,6 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
int bdrv_query_snapshot_info_list(BlockDriverState *bs,
SnapshotInfoList **p_list,
Error **errp);
-void bdrv_query_block_node_info(BlockDriverState *bs,
-BlockNodeInfo **p_info,
-Error **errp);
void bdrv_query_image_info(BlockDriverState *bs,
ImageInfo **p_info,
bool flat,
--
2.35.3
[PATCH v2 08/10] block: Don't query all block devices at hmp_nbd_server_start
We're currently doing a full query-block just to enumerate the devices
for qmp_nbd_server_add and then discarding the BlockInfoList
afterwards. Alter hmp_nbd_server_start to instead iterate explicitly
over the block_backends list.
This allows the removal of the dependency on qmp_query_block from
hmp_nbd_server_start. This is desirable because we're about to move
qmp_query_block into a coroutine and don't need to change the NBD code
at the same time.
Signed-off-by: Fabiano Rosas
---
block/monitor/block-hmp-cmds.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index ca2599de44..26116fe831 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -394,7 +394,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
bool writable = qdict_get_try_bool(qdict, "writable", false);
bool all = qdict_get_try_bool(qdict, "all", false);
Error *local_err = NULL;
-BlockInfoList *block_list, *info;
+BlockBackend *blk;
SocketAddress *addr;
NbdServerAddOptions export;
@@ -419,18 +419,24 @@ void hmp_nbd_server_start(Monitor *mon, const QDict
*qdict)
return;
}
-/* Then try adding all block devices. If one fails, close all and
+/*
+ * Then try adding all block devices. If one fails, close all and
* exit.
*/
-block_list = qmp_query_block(NULL);
+for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) {
+BlockDriverState *bs = blk_bs(blk);
-for (info = block_list; info; info = info->next) {
-if (!info->value->inserted) {
+if (!*blk_name(blk) && !blk_get_attached_dev(blk)) {
+continue;
+}
+
+bs = bdrv_skip_implicit_filters(bs);
+if (!bs || !bs->drv) {
continue;
}
export = (NbdServerAddOptions) {
-.device = info->value->device,
+.device = g_strdup(blk_name(blk)),
.has_writable = true,
.writable = writable,
};
@@ -443,8 +449,6 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
}
}
-qapi_free_BlockInfoList(block_list);
-
exit:
hmp_handle_error(mon, local_err);
}
--
2.35.3
[PATCH v2 04/10] block: Temporarily mark bdrv_co_get_allocated_file_size as mixed
Some callers of this function are about to be converted to run in coroutines, so allow it to be executed both inside and outside a coroutine while we convert all the callers. This will be reverted once all callers of bdrv_do_query_node_info run in a coroutine. Signed-off-by: Fabiano Rosas Reviewed-by: Eric Blake --- include/block/block-io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/block/block-io.h b/include/block/block-io.h index 43af816d75..f31e25cf56 100644 --- a/include/block/block-io.h +++ b/include/block/block-io.h @@ -87,7 +87,7 @@ int64_t co_wrapper_mixed_bdrv_rdlock bdrv_getlength(BlockDriverState *bs); int64_t coroutine_fn GRAPH_RDLOCK bdrv_co_get_allocated_file_size(BlockDriverState *bs); -int64_t co_wrapper_bdrv_rdlock +int64_t co_wrapper_mixed_bdrv_rdlock bdrv_get_allocated_file_size(BlockDriverState *bs); BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts, -- 2.35.3
[PATCH v2 05/10] block: Convert bdrv_query_block_graph_info to coroutine
We're converting callers of bdrv_get_allocated_file_size() to run in
coroutines because that function will be made asynchronous when called
(indirectly) from the QMP dispatcher.
This function is a candidate because it calls bdrv_do_query_node_info(),
which in turn calls bdrv_get_allocated_file_size().
All the functions called from bdrv_do_query_node_info() onwards are
coroutine-safe, either have a coroutine version themselves[1] or are
mostly simple code/string manipulation[2].
1) bdrv_getlength(), bdrv_get_allocated_file_size(), bdrv_get_info(),
bdrv_get_specific_info();
2) bdrv_refresh_filename(), bdrv_get_format_name(),
bdrv_get_full_backing_filename(), bdrv_query_snapshot_info_list();
Signed-off-by: Fabiano Rosas
---
block/qapi.c | 12 +++-
include/block/qapi.h | 6 +-
qemu-img.c | 2 --
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 1cbb0935ff..a2e71edaff 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -375,7 +375,7 @@ fail:
}
/**
- * bdrv_query_block_graph_info:
+ * bdrv_co_query_block_graph_info:
* @bs: root node to start from
* @p_info: location to store image information
* @errp: location to store error information
@@ -384,15 +384,17 @@ fail:
*
* @p_info will be set only on success. On error, store error in @errp.
*/
-void bdrv_query_block_graph_info(BlockDriverState *bs,
- BlockGraphInfo **p_info,
- Error **errp)
+void coroutine_fn bdrv_co_query_block_graph_info(BlockDriverState *bs,
+ BlockGraphInfo **p_info,
+ Error **errp)
{
BlockGraphInfo *info;
BlockChildInfoList **children_list_tail;
BdrvChild *c;
ERRP_GUARD();
+assert_bdrv_graph_readable();
+
info = g_new0(BlockGraphInfo, 1);
bdrv_do_query_node_info(bs, qapi_BlockGraphInfo_base(info), errp);
if (*errp) {
@@ -408,7 +410,7 @@ void bdrv_query_block_graph_info(BlockDriverState *bs,
QAPI_LIST_APPEND(children_list_tail, c_info);
c_info->name = g_strdup(c->name);
-bdrv_query_block_graph_info(c->bs, &c_info->info, errp);
+bdrv_co_query_block_graph_info(c->bs, &c_info->info, errp);
if (*errp) {
goto fail;
}
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 8663971c58..7035bcd1ae 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -25,6 +25,7 @@
#ifndef BLOCK_QAPI_H
#define BLOCK_QAPI_H
+#include "block/block-common.h"
#include "block/graph-lock.h"
#include "block/snapshot.h"
#include "qapi/qapi-types-block-core.h"
@@ -41,7 +42,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
bool flat,
bool skip_implicit_filters,
Error **errp);
-void GRAPH_RDLOCK
+void coroutine_fn GRAPH_RDLOCK
+bdrv_co_query_block_graph_info(BlockDriverState *bs, BlockGraphInfo **p_info,
+ Error **errp);
+void co_wrapper_bdrv_rdlock
bdrv_query_block_graph_info(BlockDriverState *bs, BlockGraphInfo **p_info,
Error **errp);
diff --git a/qemu-img.c b/qemu-img.c
index 27f48051b0..8066286f5e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2945,9 +2945,7 @@ static BlockGraphInfoList *collect_image_info_list(bool
image_opts,
* duplicate the backing chain information that we obtain by walking
* the chain manually here.
*/
-bdrv_graph_rdlock_main_loop();
bdrv_query_block_graph_info(bs, &info, &err);
-bdrv_graph_rdunlock_main_loop();
if (err) {
error_report_err(err);
--
2.35.3
[PATCH v2 10/10] block: Add a thread-pool version of fstat
From: João Silva
The fstat call can take a long time to finish when running over
NFS. Add a version of it that runs in the thread pool.
Adapt one of its users, raw_co_get_allocated_file size to use the new
version. That function is called via QMP under the qemu_global_mutex
so it has a large chance of blocking VCPU threads in case it takes too
long to finish.
Reviewed-by: Claudio Fontana
Signed-off-by: João Silva
Signed-off-by: Fabiano Rosas
---
block/file-posix.c | 40 +---
include/block/raw-aio.h | 4 +++-
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index ac1ed54811..45232dc0f9 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -227,6 +227,9 @@ typedef struct RawPosixAIOData {
struct {
unsigned long op;
} zone_mgmt;
+struct {
+struct stat *st;
+} fstat;
};
} RawPosixAIOData;
@@ -2614,6 +2617,34 @@ static void raw_close(BlockDriverState *bs)
}
}
+static int handle_aiocb_fstat(void *opaque)
+{
+RawPosixAIOData *aiocb = opaque;
+
+if (fstat(aiocb->aio_fildes, aiocb->fstat.st) < 0) {
+return -errno;
+}
+
+return 0;
+}
+
+static int coroutine_fn raw_co_fstat(BlockDriverState *bs, struct stat *st)
+{
+BDRVRawState *s = bs->opaque;
+RawPosixAIOData acb;
+
+acb = (RawPosixAIOData) {
+.bs = bs,
+.aio_fildes = s->fd,
+.aio_type = QEMU_AIO_FSTAT,
+.fstat = {
+.st = st,
+},
+};
+
+return raw_thread_pool_submit(handle_aiocb_fstat, &acb);
+}
+
/**
* Truncates the given regular file @fd to @offset and, when growing, fills the
* new space according to @prealloc.
@@ -2853,11 +2884,14 @@ static int64_t coroutine_fn
raw_co_getlength(BlockDriverState *bs)
static int64_t coroutine_fn raw_co_get_allocated_file_size(BlockDriverState
*bs)
{
struct stat st;
-BDRVRawState *s = bs->opaque;
+int ret;
-if (fstat(s->fd, &st) < 0) {
-return -errno;
+ret = raw_co_fstat(bs, &st);
+
+if (ret) {
+return ret;
}
+
return (int64_t)st.st_blocks * 512;
}
diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index 0f63c2800c..1f2c678461 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -31,6 +31,7 @@
#define QEMU_AIO_ZONE_REPORT 0x0100
#define QEMU_AIO_ZONE_MGMT0x0200
#define QEMU_AIO_ZONE_APPEND 0x0400
+#define QEMU_AIO_FSTAT0x0800
#define QEMU_AIO_TYPE_MASK \
(QEMU_AIO_READ | \
QEMU_AIO_WRITE | \
@@ -42,7 +43,8 @@
QEMU_AIO_TRUNCATE | \
QEMU_AIO_ZONE_REPORT | \
QEMU_AIO_ZONE_MGMT | \
- QEMU_AIO_ZONE_APPEND)
+ QEMU_AIO_ZONE_APPEND | \
+ QEMU_AIO_FSTAT)
/* AIO flags */
#define QEMU_AIO_MISALIGNED 0x1000
--
2.35.3
[PATCH v2 06/10] block: Convert bdrv_block_device_info into co_wrapper
We're converting callers of bdrv_get_allocated_file_size() to run in
coroutines because that function will be made asynchronous when called
(indirectly) from the QMP dispatcher.
This function is a candidate because it calls bdrv_query_image_info()
-> bdrv_do_query_node_info() -> bdrv_get_allocated_file_size().
It is safe to turn this is a coroutine because the code it calls is
made up of either simple accessors and string manipulation functions
[1] or it has already been determined to be safe [2].
1) bdrv_refresh_filename(), bdrv_is_read_only(),
blk_enable_write_cache(), bdrv_cow_bs(), blk_get_public(),
throttle_group_get_name(), bdrv_write_threshold_get(),
bdrv_query_dirty_bitmaps(), throttle_group_get_config(),
bdrv_filter_or_cow_bs(), bdrv_skip_implicit_filters()
2) bdrv_do_query_node_info() (see previous commit);
Signed-off-by: Fabiano Rosas
---
block/qapi.c | 8
include/block/qapi.h | 12
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index a2e71edaff..20660e15d6 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -41,10 +41,10 @@
#include "qemu/qemu-print.h"
#include "sysemu/block-backend.h"
-BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
-BlockDriverState *bs,
-bool flat,
-Error **errp)
+BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk,
+BlockDriverState *bs,
+bool flat,
+Error **errp)
{
ImageInfo **p_image_info;
ImageInfo *backing_info;
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 7035bcd1ae..5cb0202791 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -30,10 +30,14 @@
#include "block/snapshot.h"
#include "qapi/qapi-types-block-core.h"
-BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
-BlockDriverState *bs,
-bool flat,
-Error **errp);
+BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk,
+BlockDriverState *bs,
+bool flat,
+Error **errp);
+BlockDeviceInfo *co_wrapper bdrv_block_device_info(BlockBackend *blk,
+ BlockDriverState *bs,
+ bool flat,
+ Error **errp);
int bdrv_query_snapshot_info_list(BlockDriverState *bs,
SnapshotInfoList **p_list,
Error **errp);
--
2.35.3
[PATCH v2 00/10] block: Make raw_co_get_allocated_file_size asynchronous
Hi, The major change from the last version is that this time I'm moving all of the callers of bdrv_get_allocated_file_size() into coroutines. I had to make some temporary changes to avoid asserts while not all the callers were converted. I tried my best to explain why I think the changes are safe. To avoid changing too much of the code I added a change that removes the dependency of qmp_query_block from hmp_nbd_server_start, that way I don't need to move all of the nbd code into a coroutine as well. Based on: [PATCH v2 00/11] block: Re-enable the graph lock https://lore.kernel.org/r/20230605085711.21261-1-kw...@redhat.com changes: - fixed duplicated commit message [Lin] - clarified why we need to convert info-block [Claudio] - added my rationale of why the changes are safe [Eric] - converted all callers to coroutines [Kevin] - made hmp_nbd_server_start don't depend on qmp_query_block CI run: https://gitlab.com/farosas/qemu/-/pipelines/895525156 === v1: https://lore.kernel.org/r/20230523213903.18418-1-faro...@suse.de As discussed in another thread [1], here's an RFC addressing a VCPU softlockup encountered when issuing QMP commands that target a disk placed on NFS. Since QMP commands happen with the qemu_global_mutex locked, any command that takes too long to finish will block other threads waiting to take the global mutex. One such thread could be a VCPU thread going out of the guest to handle IO. This is the case when issuing the QMP command query-block, which eventually calls raw_co_get_allocated_file_size(). This function makes an 'fstat' call that has been observed to take a long time (seconds) over NFS. NFS latency issues aside, we can improve the situation by not blocking VCPU threads while the command is running. Move the 'fstat' call into the thread-pool and make the necessary adaptations to ensure raw_co_get_allocated_file_size runs in a coroutine in the block driver aio_context. 1- Question about QMP and BQL https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg03141.html Fabiano Rosas (8): block: Remove bdrv_query_block_node_info block: Remove unnecessary variable in bdrv_block_device_info block: Allow the wrapper script to see functions declared in qapi.h block: Temporarily mark bdrv_co_get_allocated_file_size as mixed block: Convert bdrv_query_block_graph_info to coroutine block: Convert bdrv_block_device_info into co_wrapper block: Don't query all block devices at hmp_nbd_server_start block: Convert qmp_query_block() to coroutine_fn João Silva (1): block: Add a thread-pool version of fstat Lin Ma (1): block: Convert qmp_query_named_block_nodes to coroutine block/file-posix.c | 40 +-- block/meson.build | 1 + block/monitor/block-hmp-cmds.c | 22 ++- block/qapi.c | 62 +- blockdev.c | 6 +-- hmp-commands-info.hx | 1 + include/block/block-hmp-cmds.h | 2 +- include/block/qapi.h | 17 include/block/raw-aio.h| 4 +- qapi/block-core.json | 5 ++- qemu-img.c | 2 - scripts/block-coroutine-wrapper.py | 1 + 12 files changed, 90 insertions(+), 73 deletions(-) -- 2.35.3
[PATCH v2 03/10] block: Allow the wrapper script to see functions declared in qapi.h
The following patches will add co_wrapper annotations to functions
declared in qapi.h. Add that header to the set of files used by
block-coroutine-wrapper.py.
Signed-off-by: Fabiano Rosas
---
block/meson.build | 1 +
scripts/block-coroutine-wrapper.py | 1 +
2 files changed, 2 insertions(+)
diff --git a/block/meson.build b/block/meson.build
index fb4332bd66..7ad6a396a4 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -150,6 +150,7 @@ block_gen_c = custom_target('block-gen.c',
'../include/block/dirty-bitmap.h',
'../include/block/block_int-io.h',
'../include/block/block-global-state.h',
+ '../include/block/qapi.h',
'../include/sysemu/block-backend-global-state.h',
'../include/sysemu/block-backend-io.h',
'coroutines.h'
diff --git a/scripts/block-coroutine-wrapper.py
b/scripts/block-coroutine-wrapper.py
index d4a183db61..814b62df26 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -44,6 +44,7 @@ def gen_header():
#include "block/block-gen.h"
#include "block/block_int.h"
#include "block/dirty-bitmap.h"
+#include "block/qapi.h"
"""
--
2.35.3
[PATCH v2 02/10] block: Remove unnecessary variable in bdrv_block_device_info
The commit 5d8813593f ("block/qapi: Let bdrv_query_image_info()
recurse") removed the loop where we set the 'bs0' variable, so now it
is just the same as 'bs'.
Signed-off-by: Fabiano Rosas
---
block/qapi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 79bf80c503..1cbb0935ff 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -48,7 +48,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
{
ImageInfo **p_image_info;
ImageInfo *backing_info;
-BlockDriverState *bs0, *backing;
+BlockDriverState *backing;
BlockDeviceInfo *info;
ERRP_GUARD();
@@ -145,7 +145,6 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->write_threshold = bdrv_write_threshold_get(bs);
-bs0 = bs;
p_image_info = &info->image;
info->backing_file_depth = 0;
@@ -153,7 +152,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
* Skip automatically inserted nodes that the user isn't aware of for
* query-block (blk != NULL), but not for query-named-block-nodes
*/
-bdrv_query_image_info(bs0, p_image_info, flat, blk != NULL, errp);
+bdrv_query_image_info(bs, p_image_info, flat, blk != NULL, errp);
if (*errp) {
qapi_free_BlockDeviceInfo(info);
return NULL;
--
2.35.3
Re: [PATCH 18/22] target/arm/tcg: Define SME set_svcr() helper in common 'helper.h'
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: SME set_svcr() is called by handle_msr_i() in translate-a64.c. To be able to restrict helper-sme.h.inc to SME-specific files, move the set_svcr() helper definition into the common helper.h. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h | 1 + target/arm/tcg/helper-sme.h.inc | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 05/22] target/arm/tcg: Extract iwMMXt helpers from the generic 'helper.h'
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h| 96 +--- target/arm/tcg/helper-iwmmxt.h.inc | 114 + 2 files changed, 115 insertions(+), 95 deletions(-) create mode 100644 target/arm/tcg/helper-iwmmxt.h.inc Reviewed-by: Richard Henderson r~
Re: [PATCH 15/22] target/arm/tcg: Extract SVE2 definitions to 'helper-sve.h.inc'
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: helper.h is used by all units, but not all require the SVE2 definitions. We already have helper-sve.h.inc for SVE* definitions, move them there. The next commit will remove it from the common helper.h. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h | 46 - target/arm/tcg/helper-sve.h.inc | 46 + 2 files changed, 46 insertions(+), 46 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 14/22] target/arm/tcg: Reduce 'helper-neon.h.inc' inclusion
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: Instead of including helper-neon.h.inc via helper.h which is included by all TCG files, restrict it to the few files that require it. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h | 1 - target/arm/tcg/neon_helper.c| 6 +- target/arm/tcg/translate-a64.c | 5 + target/arm/tcg/translate-neon.c | 6 ++ target/arm/tcg/translate-sme.c | 5 + target/arm/tcg/translate.c | 5 + target/arm/tcg/vec_helper.c | 5 + 7 files changed, 31 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index 2cf25f0605..82c34df538 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -608,5 +608,4 @@ DEF_HELPER_FLAGS_5(gvec_uclamp_d, TCG_CALL_NO_RWG, #include "tcg/helper-sme.h.inc" #endif -#include "tcg/helper-neon.h.inc" #include "tcg/helper-mve.h.inc" diff --git a/target/arm/tcg/neon_helper.c b/target/arm/tcg/neon_helper.c index 0a4ab3e42c..3346a897ad 100644 --- a/target/arm/tcg/neon_helper.c +++ b/target/arm/tcg/neon_helper.c @@ -9,10 +9,14 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/helper-proto.h" #include "fpu/softfloat.h" #include "vec_internal.h" +#define HELPER_H "tcg/helper-neon.h.inc" +#include "exec/helper-proto.h.inc" +#include "exec/helper-gen.h.inc" +#undef HELPER_H + Again, none of the *_helper.c need helper-gen.h.inc, only prototypes. r~
Re: [PATCH 13/22] target/arm/tcg: Extract NEON definitions to 'helper-neon.h.inc'
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: helper.h is used by all units, but not all require the NEON definitions. Move them to a new header; the next commit will remove it from the common helper.h. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h | 206 +-- target/arm/tcg/helper-neon.h.inc | 205 ++ 2 files changed, 206 insertions(+), 205 deletions(-) create mode 100644 target/arm/tcg/helper-neon.h.inc Reviewed-by: Richard Henderson r~
Re: [PATCH 12/22] target/arm/tcg: Move neon_tbl() from op_helper.c to neon_helper.c
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: neon_tbl() helper is only called by NEON files. No need to have it in the generic op_helper.c, move it with the rest of the NEON helpers. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/tcg/neon_helper.c | 22 ++ target/arm/tcg/op_helper.c | 22 -- 2 files changed, 22 insertions(+), 22 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v3 12/14] simpletrace: added simplified Analyzer2 class
> On 8 Jun 2023, at 14.41, Mads Ynddal wrote: > > From: Mads Ynddal > > By moving the dynamic argument construction to keyword-arguments, > we can remove all of the specialized handling, and streamline it. > If a tracing method wants to access these, they can define the > kwargs, or ignore it be placing `**kwargs` at the end of the > function's arguments list. > > Added deprecation warning to Analyzer class to make users aware > of the Analyzer2 class. No removal date is planned. > > Signed-off-by: Mads Ynddal > --- > scripts/simpletrace.py | 72 ++ > 1 file changed, 72 insertions(+) I noticed I'm missing Analyzer2 in __all__
Re: [PATCH 11/22] target/arm/tcg: Reduce 'helper-vfp.h.inc' inclusion
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: Instead of including helper-vfp.h.inc via helper.h which is included by all TCG files, restrict it to the few files that require it. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h | 1 - target/arm/tcg/mve_helper.c | 5 + target/arm/tcg/sve_helper.c | 4 target/arm/tcg/translate-a64.c | 5 + target/arm/tcg/translate-neon.c | 5 + target/arm/tcg/translate-vfp.c | 6 ++ target/arm/tcg/vec_helper.c | 5 + target/arm/vfp_helper.c | 5 + 8 files changed, 35 insertions(+), 1 deletion(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index 9d85675d17..4d1b7ec6b4 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -813,5 +813,4 @@ DEF_HELPER_FLAGS_5(gvec_uclamp_d, TCG_CALL_NO_RWG, #include "tcg/helper-sme.h.inc" #endif -#include "tcg/helper-vfp.h.inc" #include "tcg/helper-mve.h.inc" diff --git a/target/arm/tcg/mve_helper.c b/target/arm/tcg/mve_helper.c index 403b345ea3..9914f45b16 100644 --- a/target/arm/tcg/mve_helper.c +++ b/target/arm/tcg/mve_helper.c @@ -27,6 +27,11 @@ #include "tcg/tcg.h" #include "fpu/softfloat.h" +#define HELPER_H "tcg/helper-vfp.h.inc" +#include "exec/helper-proto.h.inc" +#include "exec/helper-gen.h.inc" +#undef HELPER_H + Again, none of the *_helper.c need helper-gen.h.inc, only prototypes. r~
Re: [PATCH 10/22] target/arm/tcg: Extract VFP definitions to 'helper-vfp.h.inc'
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: helper.h is used by all units, but not all require the VFP definitions. Move them to a new header; the next commit will remove it from the common helper.h. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h | 137 +--- target/arm/tcg/helper-vfp.h.inc | 136 +++ 2 files changed, 137 insertions(+), 136 deletions(-) create mode 100644 target/arm/tcg/helper-vfp.h.inc Reviewed-by: Richard Henderson r~
Re: [PATCH 09/22] target/arm/tcg: Reduce 'helper-iwmmxt.h.inc' inclusion
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: Instead of including helper-iwmmxt.h.inc via helper.h which is included by all TCG files, restrict it to the few files that require it. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/helper.h | 1 - target/arm/tcg/iwmmxt_helper.c| 6 +- target/arm/tcg/translate-iwmmxt.c | 6 ++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index c4a321c0ea..dd12fee107 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -949,5 +949,4 @@ DEF_HELPER_FLAGS_5(gvec_uclamp_d, TCG_CALL_NO_RWG, #include "tcg/helper-sme.h.inc" #endif -#include "tcg/helper-iwmmxt.h.inc" #include "tcg/helper-mve.h.inc" diff --git a/target/arm/tcg/iwmmxt_helper.c b/target/arm/tcg/iwmmxt_helper.c index 610b1b2103..fa84b5f672 100644 --- a/target/arm/tcg/iwmmxt_helper.c +++ b/target/arm/tcg/iwmmxt_helper.c @@ -22,7 +22,11 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/helper-proto.h" + +#define HELPER_H "tcg/helper-iwmmxt.h.inc" +#include "exec/helper-proto.h.inc" +#include "exec/helper-gen.h.inc" +#undef HELPER_H You should only require helper-proto.h.inc here. Otherwise, Reviewed-by: Richard Henderson This was exactly the sort of thing that I had in mind. r~
Re: [PATCH v4 0/1] ROM migration
On 09.06.23 18:55, Michael S. Tsirkin wrote: This is tagged, pull request will be delayed due to kvm forum though. OK, thanks! On Fri, Jun 09, 2023 at 04:32:40PM +0300, Vladimir Sementsov-Ogievskiy wrote: ping. On 22.05.23 23:17, Vladimir Sementsov-Ogievskiy wrote: v4: preparation patches are already merged to master 01: fix false-positive "error: ‘size’ may be used uninitialized", keep r-bs Vladimir Sementsov-Ogievskiy (1): pci: ROM preallocation for incoming migration hw/pci/pci.c | 79 ++-- 1 file changed, 46 insertions(+), 33 deletions(-) -- Best regards, Vladimir -- Best regards, Vladimir
Re: [PATCH 08/22] target/arm/tcg: Extract iwmmxt code to translate-iwmmxt.c
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: Extract 1300 lines from the big enough translate.c. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/tcg/translate-iwmmxt.c | 1334 + target/arm/tcg/translate.c| 1312 target/arm/tcg/meson.build|1 + 3 files changed, 1335 insertions(+), 1312 deletions(-) create mode 100644 target/arm/tcg/translate-iwmmxt.c Reviewed-by: Richard Henderson r~
Re: [PATCH] tcg/tci: Fix MemOpIdx operand index for 3-operand memops
On 6/8/23 09:58, Philippe Mathieu-Daudé wrote:
On 8/6/23 18:29, Richard Henderson wrote:
Cut and paste error from the 4-operand memops.
Fixes: ab64da797740 ("tcg/tci: Adjust passing of MemOpIdx")
I had this patch tagged for review but you were quicker
(not sure I'd have caught the issue although)
Signed-off-by: Richard Henderson
---
tcg/tci/tcg-target.c.inc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé
Applied to master, though it hasn't completely greened the board.
r~
Re: [PULL 0/1] maintainers queue
On 6/8/23 11:21, Jagannathan Raman wrote: The following changes since commit 45ae97993a75f975f1a01d25564724c7e10a543f: Merge tag 'pull-tricore-20230607' of https://github.com/bkoppelmann/qemu into staging (2023-06-07 11:45:22 -0700) are available in the Git repository at: https://gitlab.com/jraman/qemu.git tags/pull-maintainers-20230608 for you to fetch changes up to c45309f7a40083e5034fcb19e27e3c0b1b5ec6cd: maintainers: update maintainers list for vfio-user & multi-process QEMU (2023-06-08 14:16:08 -0400) maintainers: update maintainers list for vfio-user & multi-process QEMU Signed-off-by: Jagannathan Raman Jagannathan Raman (1): maintainers: update maintainers list for vfio-user & multi-process QEMU MAINTAINERS | 1 - 1 file changed, 1 deletion(-) Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate. r~
[PATCH 5/5] cmd646: move device-specific BMDMA registers to separate memory region
The aim here is to eliminate any device-specific registers from the main BMDMA
bar memory region so it can potentially be moved into the shared PCI IDE device.
For each BMDMA bus create a new cmd646-bmdma-specific memory region representing
the device-specific BMDMA registers and then map them using aliases onto the
existing BMDMAState memory region.
Signed-off-by: Mark Cave-Ayland
---
hw/ide/cmd646.c | 111 +++-
include/hw/ide/cmd646.h | 4 ++
2 files changed, 90 insertions(+), 25 deletions(-)
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 9103da581f..dd495f2e1b 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -90,7 +90,6 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
unsigned size)
{
BMDMAState *bm = opaque;
-PCIDevice *pci_dev = PCI_DEVICE(bm->pci_dev);
uint32_t val;
if (size != 1) {
@@ -101,19 +100,9 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
case 0:
val = bm->cmd;
break;
-case 1:
-val = pci_dev->config[MRDMODE];
-break;
case 2:
val = bm->status;
break;
-case 3:
-if (bm == &bm->pci_dev->bmdma[0]) {
-val = pci_dev->config[UDIDETCR0];
-} else {
-val = pci_dev->config[UDIDETCR1];
-}
-break;
default:
val = 0xff;
break;
@@ -127,7 +116,6 @@ static void bmdma_write(void *opaque, hwaddr addr,
uint64_t val, unsigned size)
{
BMDMAState *bm = opaque;
-PCIDevice *pci_dev = PCI_DEVICE(bm->pci_dev);
if (size != 1) {
return;
@@ -138,23 +126,10 @@ static void bmdma_write(void *opaque, hwaddr addr,
case 0:
bmdma_cmd_writeb(bm, val);
break;
-case 1:
-pci_dev->config[MRDMODE] =
-(pci_dev->config[MRDMODE] & ~0x30) | (val & 0x30);
-cmd646_update_dma_interrupts(pci_dev);
-cmd646_update_irq(pci_dev);
-break;
case 2:
bm->status = (val & 0x60) | (bm->status & 1) |
(bm->status & ~val & 0x06);
break;
-case 3:
-if (bm == &bm->pci_dev->bmdma[0]) {
-pci_dev->config[UDIDETCR0] = val;
-} else {
-pci_dev->config[UDIDETCR1] = val;
-}
-break;
}
}
@@ -181,6 +156,91 @@ static void bmdma_setup_bar(PCIIDEState *d)
}
}
+static uint64_t cmd646_bmdma_read(void *opaque, hwaddr addr, unsigned size)
+{
+BMDMAState *bm = opaque;
+PCIDevice *pci_dev = PCI_DEVICE(bm->pci_dev);
+uint32_t val;
+
+if (size != 1) {
+return ((uint64_t)1 << (size * 8)) - 1;
+}
+
+switch (addr & 3) {
+case 1:
+val = pci_dev->config[MRDMODE];
+break;
+case 3:
+if (bm == &bm->pci_dev->bmdma[0]) {
+val = pci_dev->config[UDIDETCR0];
+} else {
+val = pci_dev->config[UDIDETCR1];
+}
+break;
+default:
+val = 0xff;
+break;
+}
+
+trace_bmdma_read_cmd646(addr, val);
+return val;
+}
+
+static void cmd646_bmdma_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned size)
+{
+BMDMAState *bm = opaque;
+PCIDevice *pci_dev = PCI_DEVICE(bm->pci_dev);
+
+if (size != 1) {
+return;
+}
+
+trace_bmdma_write_cmd646(addr, val);
+switch (addr & 3) {
+case 1:
+pci_dev->config[MRDMODE] =
+(pci_dev->config[MRDMODE] & ~0x30) | (val & 0x30);
+cmd646_update_dma_interrupts(pci_dev);
+cmd646_update_irq(pci_dev);
+break;
+case 3:
+if (bm == &bm->pci_dev->bmdma[0]) {
+pci_dev->config[UDIDETCR0] = val;
+} else {
+pci_dev->config[UDIDETCR1] = val;
+}
+break;
+}
+}
+
+static const MemoryRegionOps cmd646_bmdma_ops = {
+.read = cmd646_bmdma_read,
+.write = cmd646_bmdma_write,
+};
+
+static void cmd646_bmdma_setup(PCIIDEState *d)
+{
+CMD646IDEState *s = CMD646_IDE(d);
+BMDMAState *bm;
+int i;
+
+/* Setup aliases for device-specific BMDMA registers */
+for (i = 0; i < 2; i++) {
+bm = &d->bmdma[i];
+memory_region_init_io(&s->bmdma_mem[i], OBJECT(d), &cmd646_bmdma_ops,
+ bm, "cmd646-bmdma", 4);
+memory_region_init_alias(&s->bmdma_mem_alias[i][0], OBJECT(d),
+ "cmd646-bmdma[1]", &s->bmdma_mem[i], 0x1, 1);
+memory_region_add_subregion(&bm->extra_io, 0x1,
+&s->bmdma_mem_alias[i][0]);
+memory_region_init_alias(&s->bmdma_mem_alias[i][1], OBJECT(d),
+ "cmd646-bmdma[3]", &s->bmdma_mem[i], 0x3, 1);
+memory_region_add_subregion(&bm->extra_io, 0x3,
+&s->bmdma_mem_alias[i][1]);
+}
+}
+
static void cmd646_update_irq(PCIDevice *pd)
{
int pci_level;
@
[PATCH 2/5] cmd646: create separate header and QOM type for CMD646_IDE
This will enable CMD646-specific fields to be added to CMD6464IDEState in
future.
Signed-off-by: Mark Cave-Ayland
---
hw/ide/cmd646.c | 4 +++-
include/hw/ide/cmd646.h | 38 ++
2 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 include/hw/ide/cmd646.h
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 20f1e41d57..a3e227fba7 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,6 +33,7 @@
#include "sysemu/reset.h"
#include "hw/ide/pci.h"
+#include "hw/ide/cmd646.h"
#include "trace.h"
/* CMD646 specific */
@@ -339,9 +340,10 @@ static void cmd646_ide_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo cmd646_ide_info = {
-.name = "cmd646-ide",
+.name = TYPE_CMD646_IDE,
.parent= TYPE_PCI_IDE,
.class_init= cmd646_ide_class_init,
+.instance_size = sizeof(CMD646IDEState),
};
static void cmd646_ide_register_types(void)
diff --git a/include/hw/ide/cmd646.h b/include/hw/ide/cmd646.h
new file mode 100644
index 00..4780b1132c
--- /dev/null
+++ b/include/hw/ide/cmd646.h
@@ -0,0 +1,38 @@
+/*
+ * QEMU IDE Emulation: PCI cmd646 support.
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * 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.
+ */
+
+#ifndef HW_IDE_CMD646_H
+#define HW_IDE_CMD646_H
+
+#define TYPE_CMD646_IDE "cmd646-ide"
+OBJECT_DECLARE_SIMPLE_TYPE(CMD646IDEState, CMD646_IDE)
+
+#include "hw/ide/pci.h"
+
+struct CMD646IDEState {
+PCIIDEState parent_obj;
+};
+
+#endif
--
2.30.2
[PATCH 3/5] cmd646: use TYPE_CMD646_IDE instead of hardcoded "cmd646-ide" string
Signed-off-by: Mark Cave-Ayland --- hw/alpha/dp264.c | 4 ++-- hw/sparc64/sun4u.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c index 03495e1e60..f2affecad9 100644 --- a/hw/alpha/dp264.c +++ b/hw/alpha/dp264.c @@ -13,7 +13,7 @@ #include "alpha_sys.h" #include "qemu/error-report.h" #include "hw/rtc/mc146818rtc.h" -#include "hw/ide/pci.h" +#include "hw/ide/cmd646.h" #include "hw/isa/superio.h" #include "net/net.h" #include "qemu/cutils.h" @@ -132,7 +132,7 @@ static void clipper_init(MachineState *machine) isa_create_simple(isa_bus, TYPE_SMC37C669_SUPERIO); /* IDE disk setup. */ -pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide"); +pci_dev = pci_create_simple(pci_bus, -1, TYPE_CMD646_IDE); pci_ide_create_devs(pci_dev); /* Load PALcode. Given that this is not "real" cpu palcode, diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index e2858a0331..66b55eabd1 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -50,7 +50,7 @@ #include "hw/sparc/sparc64.h" #include "hw/nvram/fw_cfg.h" #include "hw/sysbus.h" -#include "hw/ide/pci.h" +#include "hw/ide/cmd646.h" #include "hw/loader.h" #include "hw/fw-path-provider.h" #include "elf.h" @@ -673,7 +673,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem, qemu_macaddr_default_if_unset(&macaddr); } -pci_dev = pci_new(PCI_DEVFN(3, 0), "cmd646-ide"); +pci_dev = pci_new(PCI_DEVFN(3, 0), TYPE_CMD646_IDE); qdev_prop_set_uint32(&pci_dev->qdev, "secondary", 1); pci_realize_and_unref(pci_dev, pci_busA, &error_fatal); pci_ide_create_devs(pci_dev); -- 2.30.2
[PATCH 1/5] cmd646: checkpatch fixes
Signed-off-by: Mark Cave-Ayland
---
hw/ide/cmd646.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index a68357c1c5..20f1e41d57 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -96,7 +96,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
return ((uint64_t)1 << (size * 8)) - 1;
}
-switch(addr & 3) {
+switch (addr & 3) {
case 0:
val = bm->cmd;
break;
@@ -133,7 +133,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
}
trace_bmdma_write_cmd646(addr, val);
-switch(addr & 3) {
+switch (addr & 3) {
case 0:
bmdma_cmd_writeb(bm, val);
break;
@@ -144,7 +144,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
cmd646_update_irq(pci_dev);
break;
case 2:
-bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val &
0x06);
+bm->status = (val & 0x60) | (bm->status & 1) |
+ (bm->status & ~val & 0x06);
break;
case 3:
if (bm == &bm->pci_dev->bmdma[0]) {
@@ -167,7 +168,7 @@ static void bmdma_setup_bar(PCIIDEState *d)
int i;
memory_region_init(&d->bmdma_bar, OBJECT(d), "cmd646-bmdma", 16);
-for(i = 0;i < 2; i++) {
+for (i = 0; i < 2; i++) {
bm = &d->bmdma[i];
memory_region_init_io(&bm->extra_io, OBJECT(d), &cmd646_bmdma_ops, bm,
"cmd646-bmdma-bus", 4);
@@ -255,7 +256,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error
**errp)
pci_conf[PCI_CLASS_PROG] = 0x8f;
-pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
+pci_conf[CNTRL] = CNTRL_EN_CH0; /* enable IDE0 */
if (d->secondary) {
/* XXX: if not enabled, really disable the seconday IDE controller */
pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
@@ -289,7 +290,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error
**errp)
pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
/* TODO: RST# value should be 0 */
-pci_conf[PCI_INTERRUPT_PIN] = 0x01; // interrupt on pin 1
+pci_conf[PCI_INTERRUPT_PIN] = 0x01; /* interrupt on pin 1 */
qdev_init_gpio_in(ds, cmd646_set_irq, 2);
for (i = 0; i < 2; i++) {
--
2.30.2
[PATCH 0/5] cmd646: move device-specific BMDMA registers to separate memory region
This series follows on from a comment I made on Bernhard's PCI IDE tidy-up series [1] that it should be possible to further consolidate the BMDMA registers into the PCI IDE device with some minor rework to the CMD646 device. It does this by moving the CMD646 device-specific BMDMA registers to a separate memory region, and then aliasing the device-specific BMDMA registers from the existing BMDMAState memory region. With this in place it should be fairly trivial to extend the consolidation implementation in [1]. Before "info mtree" output: 8200-820f (prio 1, i/o): cmd646-bmdma 8200-8203 (prio 0, i/o): cmd646-bmdma-bus 8204-8207 (prio 0, i/o): cmd646-bmdma-ioport 8208-820b (prio 0, i/o): cmd646-bmdma-bus 820c-820f (prio 0, i/o): cmd646-bmdma-ioport After "info mtree" output: 8200-820f (prio 1, i/o): cmd646-bmdma 8200-8203 (prio 0, i/o): cmd646-bmdma-bus 8201-8201 (prio 0, i/o): alias cmd646-bmdma[1] @cmd646-bmdma 0001-0001 8203-8203 (prio 0, i/o): alias cmd646-bmdma[3] @cmd646-bmdma 0003-0003 8204-8207 (prio 0, i/o): cmd646-bmdma-ioport 8208-820b (prio 0, i/o): cmd646-bmdma-bus 8209-8209 (prio 0, i/o): alias cmd646-bmdma[1] @cmd646-bmdma 0001-0001 820b-820b (prio 0, i/o): alias cmd646-bmdma[3] @cmd646-bmdma 0003-0003 820c-820f (prio 0, i/o): cmd646-bmdma-ioport The series was tested by confirming that breakpoints on the CMD646-specific BMDMA registers were being hit and that my test Debian install ISO still boots under qemu-system-sparc64. Signed-off-by: Mark Cave-Ayland [1] https://patchew.org/QEMU/20230422150728.176512-1-shen...@gmail.com/ Mark Cave-Ayland (5): cmd646: checkpatch fixes cmd646: create separate header and QOM type for CMD646_IDE cmd646: use TYPE_CMD646_IDE instead of hardcoded "cmd646-ide" string cmd646: rename cmd646_bmdma_ops to bmdma_ops cmd646: move device-specific BMDMA registers to separate memory region hw/alpha/dp264.c| 4 +- hw/ide/cmd646.c | 122 ++-- hw/sparc64/sun4u.c | 4 +- include/hw/ide/cmd646.h | 42 ++ 4 files changed, 139 insertions(+), 33 deletions(-) create mode 100644 include/hw/ide/cmd646.h -- 2.30.2
[PATCH 4/5] cmd646: rename cmd646_bmdma_ops to bmdma_ops
This is to allow us to use the cmd646_bmdma_ops name for the CMD646
device-specific registers in the next commit.
Signed-off-by: Mark Cave-Ayland
---
hw/ide/cmd646.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index a3e227fba7..9103da581f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -158,7 +158,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
}
}
-static const MemoryRegionOps cmd646_bmdma_ops = {
+static const MemoryRegionOps bmdma_ops = {
.read = bmdma_read,
.write = bmdma_write,
};
@@ -171,7 +171,7 @@ static void bmdma_setup_bar(PCIIDEState *d)
memory_region_init(&d->bmdma_bar, OBJECT(d), "cmd646-bmdma", 16);
for (i = 0; i < 2; i++) {
bm = &d->bmdma[i];
-memory_region_init_io(&bm->extra_io, OBJECT(d), &cmd646_bmdma_ops, bm,
+memory_region_init_io(&bm->extra_io, OBJECT(d), &bmdma_ops, bm,
"cmd646-bmdma-bus", 4);
memory_region_add_subregion(&d->bmdma_bar, i * 8, &bm->extra_io);
memory_region_init_io(&bm->addr_ioport, OBJECT(d),
--
2.30.2
Re: [PATCH] target/riscv/vector_helper.c: clean up reference of MTYPE
On 6/8/23 02:35, Xiao Wang wrote: There's no code using MTYPE, which was a concept used in older vector implementation. Signed-off-by: Xiao Wang --- Reviewed-by: Daniel Henrique Barboza target/riscv/vector_helper.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index f261e726c2..1e06e7447c 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -378,7 +378,7 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc, /* * masked unit-stride load and store operation will be a special case of - * stride, stride = NF * sizeof (MTYPE) + * stride, stride = NF * sizeof (ETYPE) */ #define GEN_VEXT_LD_US(NAME, ETYPE, LOAD_FN)\ @@ -650,10 +650,6 @@ GEN_VEXT_LDFF(vle64ff_v, int64_t, lde_d) #define DO_MAX(N, M) ((N) >= (M) ? (N) : (M)) #define DO_MIN(N, M) ((N) >= (M) ? (M) : (N)) -/* Unsigned min/max */ -#define DO_MAXU(N, M) DO_MAX((UMTYPE)N, (UMTYPE)M) -#define DO_MINU(N, M) DO_MIN((UMTYPE)N, (UMTYPE)M) - /* * load and store whole register instructions */
[PATCH] host/include/x86_64: Use __m128i for "x" constraints
The macOS catalina compiler produces an error for __int128_t
as the type for allocation with SSE inline asm constraint.
Create a new X86Int128Union type and use the vector type for
all SSE register inputs and outputs.
Signed-off-by: Richard Henderson
---
host/include/x86_64/host/atomic128-ldst.h | 25 ---
.../x86_64/host/load-extract-al16-al8.h | 8 +++---
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/host/include/x86_64/host/atomic128-ldst.h
b/host/include/x86_64/host/atomic128-ldst.h
index adc9332f91..8d6f909d3c 100644
--- a/host/include/x86_64/host/atomic128-ldst.h
+++ b/host/include/x86_64/host/atomic128-ldst.h
@@ -8,12 +8,19 @@
* atomic primitive is meant to provide.
*/
-#ifndef AARCH64_ATOMIC128_LDST_H
-#define AARCH64_ATOMIC128_LDST_H
+#ifndef X86_64_ATOMIC128_LDST_H
+#define X86_64_ATOMIC128_LDST_H
#ifdef CONFIG_INT128_TYPE
#include "host/cpuinfo.h"
#include "tcg/debug-assert.h"
+#include
+
+typedef union {
+__m128i v;
+__int128_t i;
+Int128 s;
+} X86Int128Union;
/*
* Through clang 16, with -mcx16, __atomic_load_n is incorrectly
@@ -25,10 +32,10 @@
static inline Int128 atomic16_read_ro(const Int128 *ptr)
{
-Int128Alias r;
+X86Int128Union r;
tcg_debug_assert(HAVE_ATOMIC128_RO);
-asm("vmovdqa %1, %0" : "=x" (r.i) : "m" (*ptr));
+asm("vmovdqa %1, %0" : "=x" (r.v) : "m" (*ptr));
return r.s;
}
@@ -36,10 +43,10 @@ static inline Int128 atomic16_read_ro(const Int128 *ptr)
static inline Int128 atomic16_read_rw(Int128 *ptr)
{
__int128_t *ptr_align = __builtin_assume_aligned(ptr, 16);
-Int128Alias r;
+X86Int128Union r;
if (HAVE_ATOMIC128_RO) {
-asm("vmovdqa %1, %0" : "=x" (r.i) : "m" (*ptr_align));
+asm("vmovdqa %1, %0" : "=x" (r.v) : "m" (*ptr_align));
} else {
r.i = __sync_val_compare_and_swap_16(ptr_align, 0, 0);
}
@@ -49,10 +56,10 @@ static inline Int128 atomic16_read_rw(Int128 *ptr)
static inline void atomic16_set(Int128 *ptr, Int128 val)
{
__int128_t *ptr_align = __builtin_assume_aligned(ptr, 16);
-Int128Alias new = { .s = val };
+X86Int128Union new = { .s = val };
if (HAVE_ATOMIC128_RO) {
-asm("vmovdqa %1, %0" : "=m"(*ptr_align) : "x" (new.i));
+asm("vmovdqa %1, %0" : "=m"(*ptr_align) : "x" (new.v));
} else {
__int128_t old;
do {
@@ -65,4 +72,4 @@ static inline void atomic16_set(Int128 *ptr, Int128 val)
#include "host/include/generic/host/atomic128-ldst.h"
#endif
-#endif /* AARCH64_ATOMIC128_LDST_H */
+#endif /* X86_64_ATOMIC128_LDST_H */
diff --git a/host/include/x86_64/host/load-extract-al16-al8.h
b/host/include/x86_64/host/load-extract-al16-al8.h
index 31b6fe8c45..baa506b7b5 100644
--- a/host/include/x86_64/host/load-extract-al16-al8.h
+++ b/host/include/x86_64/host/load-extract-al16-al8.h
@@ -9,7 +9,7 @@
#define X86_64_LOAD_EXTRACT_AL16_AL8_H
#ifdef CONFIG_INT128_TYPE
-#include "host/cpuinfo.h"
+#include "host/atomic128-ldst.h"
/**
* load_atom_extract_al16_or_al8:
@@ -26,7 +26,7 @@ load_atom_extract_al16_or_al8(void *pv, int s)
uintptr_t pi = (uintptr_t)pv;
__int128_t *ptr_align = (__int128_t *)(pi & ~7);
int shr = (pi & 7) * 8;
-Int128Alias r;
+X86Int128Union r;
/*
* ptr_align % 16 is now only 0 or 8.
@@ -35,9 +35,9 @@ load_atom_extract_al16_or_al8(void *pv, int s)
* when ptr_align % 16 == 0 for 16-byte atomicity.
*/
if ((cpuinfo & CPUINFO_ATOMIC_VMOVDQU) || (pi & 8)) {
-asm("vmovdqu %1, %0" : "=x" (r.i) : "m" (*ptr_align));
+asm("vmovdqu %1, %0" : "=x" (r.v) : "m" (*ptr_align));
} else {
-asm("vmovdqa %1, %0" : "=x" (r.i) : "m" (*ptr_align));
+asm("vmovdqa %1, %0" : "=x" (r.v) : "m" (*ptr_align));
}
return int128_getlo(int128_urshift(r.s, shr));
}
--
2.34.1
Re: [PATCH 07/22] target/arm/tcg: Expose some iwmmxt methods in 'translate.h'
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote: Expose a few methods and variables before extracting iwmmxt code from translate.c. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/tcg/translate.h | 6 ++ target/arm/tcg/translate.c | 8 2 files changed, 10 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson
Re: [PATCH 06/22] target/arm/tcg: Fix iwmmxt-related code style
On 6/9/23 03:47, Philippe Mathieu-Daudé wrote:
In order to be able to move the iwmmxt-related code,
fix its style to avoid:
ERROR: braces {} are necessary for all arms of this statement
ERROR: space prohibited before that '++' (ctx:WxB)
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/tcg/translate.c | 227 +++--
1 file changed, 141 insertions(+), 86 deletions(-)
Reviewed-by: Richard Henderson
r~
Re: [PATCH 03/22] target/arm/tcg: Remove CONFIG_TCG #ifdef'ry check
On 6/9/23 03:46, Philippe Mathieu-Daudé wrote: CONFIG_TCG is always defined within target/arm/tcg/. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/tcg/cpu32.c| 4 +--- target/arm/tcg/m_helper.c | 6 -- 2 files changed, 1 insertion(+), 9 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 02/22] accel/tcg/cpu-exec: Use generic 'helper-proto-common.h' header
On 6/9/23 03:46, Philippe Mathieu-Daudé wrote: We only need lookup_tb_ptr() typedef. Signed-off-by: Philippe Mathieu-Daudé --- accel/tcg/cpu-exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) prototype, not typedef. Reviewed-by: Richard Henderson r~
Re: [PATCH 01/22] plugins: Remove unused 'exec/helper-proto.h' header
On 6/9/23 03:46, Philippe Mathieu-Daudé wrote: Signed-off-by: Philippe Mathieu-Daudé --- plugins/core.c | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] host/include/x86_64: Use __m128i for "x" constraints
On Fri, 9 Jun 2023 at 18:58, Richard Henderson wrote: > > The macOS catalina compiler produces an error for __int128_t > as the type for allocation with SSE inline asm constraint. > Create a new X86Int128Union type and use the vector type for > all SSE register inputs and outputs. > > Signed-off-by: Richard Henderson > --- > host/include/x86_64/host/atomic128-ldst.h | 25 --- > .../x86_64/host/load-extract-al16-al8.h | 8 +++--- > 2 files changed, 20 insertions(+), 13 deletions(-) Yep, this fixes the build on my x86 macos box; thanks! Tested-by: Peter Maydell -- PMM
Re: [PATCH] target/arm: Return correct result for LDG when ATA=0
On 6/9/23 08:54, Peter Maydell wrote:
The LDG instruction loads the tag from a memory address (identified
by [Xn + offset]), and then merges that tag into the destination
register Xt. We implemented this correctly for the case when
allocation tags are enabled, but didn't get it right when ATA=0:
instead of merging the tag bits into Xt, we merged them into the
memory address [Xn + offset] and then set Xt to that.
Merge the tag bits into the old Xt value, as they should be.
Cc:qemu-sta...@nongnu.org
Fixes: c15294c1e36a7dd9b25 ("target/arm: Implement LDG, STG, ST2G instructions")
Signed-off-by: Peter Maydell
---
Spotted this while I was doing decodetree conversion stuff.
I guess nobody runs with MTE available but not turned on :-)
Indeed. :-)
Reviewed-by: Richard Henderson
r~
[PATCH v3 0/8] Implement Most ARMv8.3 Pointer Authentication Features
Changes from v2 of this patchset [0]: - Remove properties for EPAC, Pauth2, FPAC, FPACCombined - Separate out aa64isar2 addition into its own patch - Comment clarifications - Several code formatting/simplifications - Rebase on top of latest upstream changes (for example, those which reorganized decoding PAC branch instructions) [0] - https://lists.nongnu.org/archive/html/qemu-devel/2023-02/msg06494.html Aaron Lindsay (8): target/arm: Add ID_AA64ISAR2_EL1 target/arm: v8.3 PAC ID_AA64ISAR[12] feature-detection target/arm: Implement v8.3 QARMA3 PAC cipher target/arm: Implement v8.3 EnhancedPAC target/arm: Implement v8.3 Pauth2 targer/arm: Inform helpers whether a PAC instruction is 'combined' target/arm: Implement v8.3 FPAC and FPACCOMBINE target/arm: Add CPU property for QARMA3, enable FPACCombined by default target/arm/cpu.h | 67 +++- target/arm/cpu64.c | 48 ++--- target/arm/helper.c| 4 +- target/arm/hvf/hvf.c | 1 + target/arm/kvm64.c | 2 + target/arm/syndrome.h | 7 ++ target/arm/tcg/helper-a64.h| 4 + target/arm/tcg/pauth_helper.c | 189 ++--- target/arm/tcg/translate-a64.c | 12 +-- 9 files changed, 270 insertions(+), 64 deletions(-) -- 2.25.1
[PATCH v3 2/8] target/arm: v8.3 PAC ID_AA64ISAR[12] feature-detection
Signed-off-by: Aaron Lindsay
---
target/arm/cpu.h | 65 +--
target/arm/tcg/pauth_helper.c | 2 +-
2 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index df04c9a9ab..22dd898577 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3714,18 +3714,77 @@ static inline bool isar_feature_aa64_pauth(const
ARMISARegisters *id)
(FIELD_DP64(0, ID_AA64ISAR1, APA, 0xf) |
FIELD_DP64(0, ID_AA64ISAR1, API, 0xf) |
FIELD_DP64(0, ID_AA64ISAR1, GPA, 0xf) |
- FIELD_DP64(0, ID_AA64ISAR1, GPI, 0xf))) != 0;
+ FIELD_DP64(0, ID_AA64ISAR1, GPI, 0xf))) != 0 ||
+ (id->id_aa64isar2 &
+(FIELD_DP64(0, ID_AA64ISAR2, APA3, 0xf) |
+ FIELD_DP64(0, ID_AA64ISAR2, GPA3, 0xf))) != 0;
}
-static inline bool isar_feature_aa64_pauth_arch(const ARMISARegisters *id)
+static inline bool isar_feature_aa64_pauth_arch_qarma5(const ARMISARegisters
*id)
{
/*
- * Return true if pauth is enabled with the architected QARMA algorithm.
+ * Return true if pauth is enabled with the architected QARMA5 algorithm.
* QEMU will always set APA+GPA to the same value.
*/
return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA) != 0;
}
+static inline bool isar_feature_aa64_pauth_arch_qarma3(const ARMISARegisters
*id)
+{
+/*
+ * Return true if pauth is enabled with the architected QARMA3 algorithm.
+ * QEMU will always set APA3+GPA3 to the same result.
+ */
+return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3) != 0;
+}
+
+static inline bool isar_feature_aa64_pauth_arch(const ARMISARegisters *id)
+{
+return isar_feature_aa64_pauth_arch_qarma5(id) ||
+isar_feature_aa64_pauth_arch_qarma3(id);
+}
+
+static inline int isar_feature_pauth_get_features(const ARMISARegisters *id)
+{
+if (isar_feature_aa64_pauth_arch_qarma5(id)) {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA);
+} else if (isar_feature_aa64_pauth_arch_qarma3(id)) {
+return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, APA3);
+} else {
+return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, API);
+}
+}
+
+static inline bool isar_feature_aa64_pauth_epac(const ARMISARegisters *id)
+{
+/*
+ * Note that unlike most AArch64 features, EPAC is treated (in the ARM
+ * psedocode, at least) as not being implemented by larger values of this
+ * field. Our usage of '>=' rather than '==' here causes our implementation
+ * of PAC logic to diverge from ARM pseudocode - we must check that
+ * isar_feature_aa64_pauth2() returns false AND
+ * isar_feature_aa64_pauth_epac() returns true, where the pseudocode reads
+ * as if EPAC is not implemented if the value of this register is > 0b10.
+ * See the implementation of pauth_addpac() for an example.
+ */
+return isar_feature_pauth_get_features(id) >= 0b0010;
+}
+
+static inline bool isar_feature_aa64_pauth2(const ARMISARegisters *id)
+{
+return isar_feature_pauth_get_features(id) >= 0b0011;
+}
+
+static inline bool isar_feature_aa64_fpac(const ARMISARegisters *id)
+{
+return isar_feature_pauth_get_features(id) >= 0b0100;
+}
+
+static inline bool isar_feature_aa64_fpac_combine(const ARMISARegisters *id)
+{
+return isar_feature_pauth_get_features(id) >= 0b0101;
+}
+
static inline bool isar_feature_aa64_tlbirange(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TLB) == 2;
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index 62af569341..3ff4610a26 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -282,7 +282,7 @@ static uint64_t pauth_computepac_impdef(uint64_t data,
uint64_t modifier,
static uint64_t pauth_computepac(CPUARMState *env, uint64_t data,
uint64_t modifier, ARMPACKey key)
{
-if (cpu_isar_feature(aa64_pauth_arch, env_archcpu(env))) {
+if (cpu_isar_feature(aa64_pauth_arch_qarma5, env_archcpu(env))) {
return pauth_computepac_architected(data, modifier, key);
} else {
return pauth_computepac_impdef(data, modifier, key);
--
2.25.1
[PATCH v3 7/8] target/arm: Implement v8.3 FPAC and FPACCOMBINE
Signed-off-by: Aaron Lindsay
---
target/arm/syndrome.h | 7 +++
target/arm/tcg/pauth_helper.c | 16
2 files changed, 23 insertions(+)
diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h
index d27d1bc31f..bf79c539d9 100644
--- a/target/arm/syndrome.h
+++ b/target/arm/syndrome.h
@@ -49,6 +49,7 @@ enum arm_exception_class {
EC_SYSTEMREGISTERTRAP = 0x18,
EC_SVEACCESSTRAP = 0x19,
EC_ERETTRAP = 0x1a,
+EC_PACFAIL= 0x1c,
EC_SMETRAP= 0x1d,
EC_INSNABORT = 0x20,
EC_INSNABORT_SAME_EL = 0x21,
@@ -231,6 +232,12 @@ static inline uint32_t syn_smetrap(SMEExceptionType etype,
bool is_16bit)
| (is_16bit ? 0 : ARM_EL_IL) | etype;
}
+static inline uint32_t syn_pacfail(bool data, int keynumber)
+{
+int error_code = (data << 1) | keynumber;
+return (EC_PACFAIL << ARM_EL_EC_SHIFT) | ARM_EL_IL | error_code;
+}
+
static inline uint32_t syn_pactrap(void)
{
return EC_PACTRAP << ARM_EL_EC_SHIFT;
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index 278d6d36bc..f42945257f 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -395,6 +395,13 @@ static uint64_t pauth_original_ptr(uint64_t ptr,
ARMVAParameters param)
}
}
+static G_NORETURN
+void pauth_fail_exception(CPUARMState *env, bool data, int keynumber,
uintptr_t ra)
+{
+int target_el = exception_target_el(env);
+raise_exception_ra(env, EXCP_UDEF, syn_pacfail(data, keynumber),
target_el, ra);
+}
+
static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr, uint64_t modifier,
ARMPACKey *key, bool data, int keynumber,
uintptr_t ra, bool is_combined)
@@ -414,6 +421,15 @@ static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr,
uint64_t modifier,
uint64_t xor_mask = MAKE_64BIT_MASK(bot_bit, top_bit - bot_bit + 1) &
~MAKE_64BIT_MASK(55, 1);
result = ptr ^ (pac & xor_mask);
+if (cpu_isar_feature(aa64_fpac_combine, cpu)
+|| (cpu_isar_feature(aa64_fpac, cpu) && !is_combined)) {
+int fpac_top = param.tbi ? 55 : 64;
+uint64_t fpac_mask = MAKE_64BIT_MASK(bot_bit, fpac_top - bot_bit);
+test = (result ^ sextract64(result, 55, 1)) & fpac_mask;
+if (unlikely(test)) {
+pauth_fail_exception(env, data, keynumber, ra);
+}
+}
} else {
test = (pac ^ ptr) & ~MAKE_64BIT_MASK(55, 1);
if (unlikely(extract64(test, bot_bit, top_bit - bot_bit))) {
--
2.25.1
[PATCH v3 3/8] target/arm: Implement v8.3 QARMA3 PAC cipher
Signed-off-by: Aaron Lindsay
Reviewed-by: Peter Maydell
Reviewed-by: Richard Henderson
---
target/arm/tcg/pauth_helper.c | 54 ---
1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index 3ff4610a26..68942015e1 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -96,6 +96,21 @@ static uint64_t pac_sub(uint64_t i)
return o;
}
+static uint64_t pac_sub1(uint64_t i)
+{
+static const uint8_t sub1[16] = {
+0xa, 0xd, 0xe, 0x6, 0xf, 0x7, 0x3, 0x5,
+0x9, 0x8, 0x0, 0xc, 0xb, 0x1, 0x2, 0x4,
+};
+uint64_t o = 0;
+int b;
+
+for (b = 0; b < 64; b += 4) {
+o |= (uint64_t)sub1[(i >> b) & 0xf] << b;
+}
+return o;
+}
+
static uint64_t pac_inv_sub(uint64_t i)
{
static const uint8_t inv_sub[16] = {
@@ -209,7 +224,7 @@ static uint64_t tweak_inv_shuffle(uint64_t i)
}
static uint64_t pauth_computepac_architected(uint64_t data, uint64_t modifier,
- ARMPACKey key)
+ ARMPACKey key, bool isqarma3)
{
static const uint64_t RC[5] = {
0xull,
@@ -219,6 +234,7 @@ static uint64_t pauth_computepac_architected(uint64_t data,
uint64_t modifier,
0x452821E638D01377ull,
};
const uint64_t alpha = 0xC0AC29B7C97C50DDull;
+int iterations = isqarma3 ? 2 : 4;
/*
* Note that in the ARM pseudocode, key0 contains bits <127:64>
* and key1 contains bits <63:0> of the 128-bit key.
@@ -231,7 +247,7 @@ static uint64_t pauth_computepac_architected(uint64_t data,
uint64_t modifier,
runningmod = modifier;
workingval = data ^ key0;
-for (i = 0; i <= 4; ++i) {
+for (i = 0; i <= iterations; ++i) {
roundkey = key1 ^ runningmod;
workingval ^= roundkey;
workingval ^= RC[i];
@@ -239,32 +255,48 @@ static uint64_t pauth_computepac_architected(uint64_t
data, uint64_t modifier,
workingval = pac_cell_shuffle(workingval);
workingval = pac_mult(workingval);
}
-workingval = pac_sub(workingval);
+if (isqarma3) {
+workingval = pac_sub1(workingval);
+} else {
+workingval = pac_sub(workingval);
+}
runningmod = tweak_shuffle(runningmod);
}
roundkey = modk0 ^ runningmod;
workingval ^= roundkey;
workingval = pac_cell_shuffle(workingval);
workingval = pac_mult(workingval);
-workingval = pac_sub(workingval);
+if (isqarma3) {
+workingval = pac_sub1(workingval);
+} else {
+workingval = pac_sub(workingval);
+}
workingval = pac_cell_shuffle(workingval);
workingval = pac_mult(workingval);
workingval ^= key1;
workingval = pac_cell_inv_shuffle(workingval);
-workingval = pac_inv_sub(workingval);
+if (isqarma3) {
+workingval = pac_sub1(workingval);
+} else {
+workingval = pac_inv_sub(workingval);
+}
workingval = pac_mult(workingval);
workingval = pac_cell_inv_shuffle(workingval);
workingval ^= key0;
workingval ^= runningmod;
-for (i = 0; i <= 4; ++i) {
-workingval = pac_inv_sub(workingval);
-if (i < 4) {
+for (i = 0; i <= iterations; ++i) {
+if (isqarma3) {
+workingval = pac_sub1(workingval);
+} else {
+workingval = pac_inv_sub(workingval);
+}
+if (i < iterations) {
workingval = pac_mult(workingval);
workingval = pac_cell_inv_shuffle(workingval);
}
runningmod = tweak_inv_shuffle(runningmod);
roundkey = key1 ^ runningmod;
-workingval ^= RC[4 - i];
+workingval ^= RC[iterations - i];
workingval ^= roundkey;
workingval ^= alpha;
}
@@ -283,7 +315,9 @@ static uint64_t pauth_computepac(CPUARMState *env, uint64_t
data,
uint64_t modifier, ARMPACKey key)
{
if (cpu_isar_feature(aa64_pauth_arch_qarma5, env_archcpu(env))) {
-return pauth_computepac_architected(data, modifier, key);
+return pauth_computepac_architected(data, modifier, key, false);
+} else if (cpu_isar_feature(aa64_pauth_arch_qarma3, env_archcpu(env))) {
+return pauth_computepac_architected(data, modifier, key, true);
} else {
return pauth_computepac_impdef(data, modifier, key);
}
--
2.25.1
[PATCH v3 5/8] target/arm: Implement v8.3 Pauth2
Signed-off-by: Aaron Lindsay
Reviewed-by: Peter Maydell
Reviewed-by: Richard Henderson
---
target/arm/tcg/pauth_helper.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index 1e9159c313..b0282d1a05 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -352,7 +352,9 @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t
ptr, uint64_t modifier,
*/
test = sextract64(ptr, bot_bit, top_bit - bot_bit);
if (test != 0 && test != -1) {
-if (cpu_isar_feature(aa64_pauth_epac, cpu)) {
+if (cpu_isar_feature(aa64_pauth2, cpu)) {
+/* No action required */
+} else if (cpu_isar_feature(aa64_pauth_epac, cpu)) {
pac = 0;
} else {
/*
@@ -367,6 +369,9 @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t
ptr, uint64_t modifier,
* Preserve the determination between upper and lower at bit 55,
* and insert pointer authentication code.
*/
+if (cpu_isar_feature(aa64_pauth2, cpu)) {
+pac ^= ptr;
+}
if (param.tbi) {
ptr &= ~MAKE_64BIT_MASK(bot_bit, 55 - bot_bit + 1);
pac &= MAKE_64BIT_MASK(bot_bit, 54 - bot_bit + 1);
@@ -393,26 +398,34 @@ static uint64_t pauth_original_ptr(uint64_t ptr,
ARMVAParameters param)
static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr, uint64_t modifier,
ARMPACKey *key, bool data, int keynumber)
{
+ARMCPU *cpu = env_archcpu(env);
ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
ARMVAParameters param = aa64_va_parameters(env, ptr, mmu_idx, data, false);
int bot_bit, top_bit;
-uint64_t pac, orig_ptr, test;
+uint64_t pac, orig_ptr, test, result;
orig_ptr = pauth_original_ptr(ptr, param);
pac = pauth_computepac(env, orig_ptr, modifier, *key);
bot_bit = 64 - param.tsz;
top_bit = 64 - 8 * param.tbi;
-test = (pac ^ ptr) & ~MAKE_64BIT_MASK(55, 1);
-if (unlikely(extract64(test, bot_bit, top_bit - bot_bit))) {
-int error_code = (keynumber << 1) | (keynumber ^ 1);
-if (param.tbi) {
-return deposit64(orig_ptr, 53, 2, error_code);
-} else {
-return deposit64(orig_ptr, 61, 2, error_code);
+if (cpu_isar_feature(aa64_pauth2, cpu)) {
+uint64_t xor_mask = MAKE_64BIT_MASK(bot_bit, top_bit - bot_bit + 1) &
+~MAKE_64BIT_MASK(55, 1);
+result = ptr ^ (pac & xor_mask);
+} else {
+test = (pac ^ ptr) & ~MAKE_64BIT_MASK(55, 1);
+if (unlikely(extract64(test, bot_bit, top_bit - bot_bit))) {
+int error_code = (keynumber << 1) | (keynumber ^ 1);
+if (param.tbi) {
+return deposit64(orig_ptr, 53, 2, error_code);
+} else {
+return deposit64(orig_ptr, 61, 2, error_code);
+}
}
+result = orig_ptr;
}
-return orig_ptr;
+return result;
}
static uint64_t pauth_strip(CPUARMState *env, uint64_t ptr, bool data)
--
2.25.1
[PATCH v3 8/8] target/arm: Add CPU property for QARMA3, enable FPACCombined by default
Signed-off-by: Aaron Lindsay
---
target/arm/cpu.h | 1 +
target/arm/cpu64.c | 48 +++---
2 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 22dd898577..0c4c6c9c82 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1061,6 +1061,7 @@ struct ArchCPU {
*/
bool prop_pauth;
bool prop_pauth_impdef;
+bool prop_pauth_qarma3;
bool prop_lpa2;
/* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 6eaf8e32cf..b0a5af7a31 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -473,9 +473,6 @@ void aarch64_add_sme_properties(Object *obj)
void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
{
-int arch_val = 0, impdef_val = 0;
-uint64_t t;
-
/* Exit early if PAuth is enabled, and fall through to disable it */
if ((kvm_enabled() || hvf_enabled()) && cpu->prop_pauth) {
if (!cpu_isar_feature(aa64_pauth, cpu)) {
@@ -486,30 +483,50 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
return;
}
-/* TODO: Handle HaveEnhancedPAC, HaveEnhancedPAC2, HaveFPAC. */
+/* Write the features into the correct field for the algorithm in use */
if (cpu->prop_pauth) {
+uint64_t t;
+
+if (cpu->prop_pauth_impdef && cpu->prop_pauth_qarma3) {
+error_setg(errp, "Cannot set both qarma3 ('pauth-qarma3') and "
+"impdef ('pauth-impdef') pointer authentication ciphers");
+return;
+}
+
+/* Implement FEAT_FPACCOMBINE for address authentication and enable
+ * generic authentication for the chosen cipher.
+ */
+int address_auth = 0b0101;
+int generic_auth = 0b0001;
+
if (cpu->prop_pauth_impdef) {
-impdef_val = 1;
+t = cpu->isar.id_aa64isar1;
+t = FIELD_DP64(t, ID_AA64ISAR1, API, address_auth);
+t = FIELD_DP64(t, ID_AA64ISAR1, GPI, generic_auth);
+cpu->isar.id_aa64isar1 = t;
+} else if (cpu->prop_pauth_qarma3) {
+t = cpu->isar.id_aa64isar2;
+t = FIELD_DP64(t, ID_AA64ISAR2, APA3, address_auth);
+t = FIELD_DP64(t, ID_AA64ISAR2, GPA3, generic_auth);
+cpu->isar.id_aa64isar2 = t;
} else {
-arch_val = 1;
+t = cpu->isar.id_aa64isar1;
+t = FIELD_DP64(t, ID_AA64ISAR1, APA, address_auth);
+t = FIELD_DP64(t, ID_AA64ISAR1, GPA, generic_auth);
+cpu->isar.id_aa64isar1 = t;
}
-} else if (cpu->prop_pauth_impdef) {
-error_setg(errp, "cannot enable pauth-impdef without pauth");
+} else if (cpu->prop_pauth_impdef || cpu->prop_pauth_qarma3) {
+error_setg(errp, "cannot enable pauth-impdef or pauth-qarma3 without
pauth");
error_append_hint(errp, "Add pauth=on to the CPU property list.\n");
}
-
-t = cpu->isar.id_aa64isar1;
-t = FIELD_DP64(t, ID_AA64ISAR1, APA, arch_val);
-t = FIELD_DP64(t, ID_AA64ISAR1, GPA, arch_val);
-t = FIELD_DP64(t, ID_AA64ISAR1, API, impdef_val);
-t = FIELD_DP64(t, ID_AA64ISAR1, GPI, impdef_val);
-cpu->isar.id_aa64isar1 = t;
}
static Property arm_cpu_pauth_property =
DEFINE_PROP_BOOL("pauth", ARMCPU, prop_pauth, true);
static Property arm_cpu_pauth_impdef_property =
DEFINE_PROP_BOOL("pauth-impdef", ARMCPU, prop_pauth_impdef, false);
+static Property arm_cpu_pauth_qarma3_property =
+DEFINE_PROP_BOOL("pauth-qarma3", ARMCPU, prop_pauth_qarma3, false);
void aarch64_add_pauth_properties(Object *obj)
{
@@ -529,6 +546,7 @@ void aarch64_add_pauth_properties(Object *obj)
cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu);
} else {
qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
+qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_qarma3_property);
}
}
--
2.25.1
[PATCH v3 4/8] target/arm: Implement v8.3 EnhancedPAC
Signed-off-by: Aaron Lindsay
Reviewed-by: Peter Maydell
Reviewed-by: Richard Henderson
---
target/arm/tcg/pauth_helper.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index 68942015e1..1e9159c313 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -326,6 +326,7 @@ static uint64_t pauth_computepac(CPUARMState *env, uint64_t
data,
static uint64_t pauth_addpac(CPUARMState *env, uint64_t ptr, uint64_t modifier,
ARMPACKey *key, bool data)
{
+ARMCPU *cpu = env_archcpu(env);
ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
ARMVAParameters param = aa64_va_parameters(env, ptr, mmu_idx, data, false);
uint64_t pac, ext_ptr, ext, test;
@@ -351,11 +352,15 @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t
ptr, uint64_t modifier,
*/
test = sextract64(ptr, bot_bit, top_bit - bot_bit);
if (test != 0 && test != -1) {
-/*
- * Note that our top_bit is one greater than the pseudocode's
- * version, hence "- 2" here.
- */
-pac ^= MAKE_64BIT_MASK(top_bit - 2, 1);
+if (cpu_isar_feature(aa64_pauth_epac, cpu)) {
+pac = 0;
+} else {
+/*
+ * Note that our top_bit is one greater than the pseudocode's
+ * version, hence "- 2" here.
+ */
+pac ^= MAKE_64BIT_MASK(top_bit - 2, 1);
+}
}
/*
--
2.25.1
[PATCH v3 1/8] target/arm: Add ID_AA64ISAR2_EL1
Signed-off-by: Aaron Lindsay
---
target/arm/cpu.h | 1 +
target/arm/helper.c | 4 ++--
target/arm/hvf/hvf.c | 1 +
target/arm/kvm64.c | 2 ++
4 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 36c608f0e6..df04c9a9ab 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1023,6 +1023,7 @@ struct ArchCPU {
uint32_t dbgdevid1;
uint64_t id_aa64isar0;
uint64_t id_aa64isar1;
+uint64_t id_aa64isar2;
uint64_t id_aa64pfr0;
uint64_t id_aa64pfr1;
uint64_t id_aa64mmfr0;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d4bee43bd0..4ced2f71e5 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8204,11 +8204,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.access = PL1_R, .type = ARM_CP_CONST,
.accessfn = access_aa64_tid3,
.resetvalue = cpu->isar.id_aa64isar1 },
-{ .name = "ID_AA64ISAR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
+{ .name = "ID_AA64ISAR2_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 2,
.access = PL1_R, .type = ARM_CP_CONST,
.accessfn = access_aa64_tid3,
- .resetvalue = 0 },
+ .resetvalue = cpu->isar.id_aa64isar2 },
{ .name = "ID_AA64ISAR3_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 3,
.access = PL1_R, .type = ARM_CP_CONST,
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 8f72624586..bf567b24db 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -847,6 +847,7 @@ static bool
hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
{ HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 },
{ HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 },
{ HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 },
+{ HV_SYS_REG_ID_AA64ISAR2_EL1, &host_isar.id_aa64isar2 },
{ HV_SYS_REG_ID_AA64MMFR0_EL1, &host_isar.id_aa64mmfr0 },
{ HV_SYS_REG_ID_AA64MMFR1_EL1, &host_isar.id_aa64mmfr1 },
{ HV_SYS_REG_ID_AA64MMFR2_EL1, &host_isar.id_aa64mmfr2 },
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 94bbd9661f..e2d05d7fc0 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -306,6 +306,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
ARM64_SYS_REG(3, 0, 0, 6, 0));
err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64isar1,
ARM64_SYS_REG(3, 0, 0, 6, 1));
+err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64isar2,
+ ARM64_SYS_REG(3, 0, 0, 6, 2));
err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr0,
ARM64_SYS_REG(3, 0, 0, 7, 0));
err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr1,
--
2.25.1
[PATCH v3 6/8] targer/arm: Inform helpers whether a PAC instruction is 'combined'
An instruction is a 'combined' Pointer Authentication instruction if it
does something in addition to PAC - for instance, branching to or
loading an address from the authenticated pointer. Knowing whether a PAC
operation is 'combined' is needed to implement the FPACCOMBINE feature
for ARMv8.3.
Signed-off-by: Aaron Lindsay
Reviewed-by: Richard Henderson
---
target/arm/tcg/helper-a64.h| 4 ++
target/arm/tcg/pauth_helper.c | 71 +++---
target/arm/tcg/translate-a64.c | 12 +++---
3 files changed, 68 insertions(+), 19 deletions(-)
diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h
index 3d5957c11f..57cfd68569 100644
--- a/target/arm/tcg/helper-a64.h
+++ b/target/arm/tcg/helper-a64.h
@@ -90,9 +90,13 @@ DEF_HELPER_FLAGS_3(pacda, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(pacdb, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(pacga, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(autia, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(autia_combined, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(autib, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(autib_combined, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(autda, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(autda_combined, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(autdb_combined, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index b0282d1a05..278d6d36bc 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -396,7 +396,8 @@ static uint64_t pauth_original_ptr(uint64_t ptr,
ARMVAParameters param)
}
static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr, uint64_t modifier,
- ARMPACKey *key, bool data, int keynumber)
+ ARMPACKey *key, bool data, int keynumber,
+ uintptr_t ra, bool is_combined)
{
ARMCPU *cpu = env_archcpu(env);
ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
@@ -518,44 +519,88 @@ uint64_t HELPER(pacga)(CPUARMState *env, uint64_t x,
uint64_t y)
return pac & 0xull;
}
-uint64_t HELPER(autia)(CPUARMState *env, uint64_t x, uint64_t y)
+static uint64_t pauth_autia(CPUARMState *env, uint64_t x, uint64_t y,
+uintptr_t ra, bool is_combined)
{
int el = arm_current_el(env);
if (!pauth_key_enabled(env, el, SCTLR_EnIA)) {
return x;
}
-pauth_check_trap(env, el, GETPC());
-return pauth_auth(env, x, y, &env->keys.apia, false, 0);
+pauth_check_trap(env, el, ra);
+return pauth_auth(env, x, y, &env->keys.apia, false, 0, ra, is_combined);
}
-uint64_t HELPER(autib)(CPUARMState *env, uint64_t x, uint64_t y)
+uint64_t HELPER(autia)(CPUARMState *env, uint64_t x, uint64_t y)
+{
+return pauth_autia(env, x, y, GETPC(), false);
+}
+
+uint64_t HELPER(autia_combined)(CPUARMState *env, uint64_t x, uint64_t y)
+{
+return pauth_autia(env, x, y, GETPC(), true);
+}
+
+static uint64_t pauth_autib(CPUARMState *env, uint64_t x, uint64_t y,
+uintptr_t ra, bool is_combined)
{
int el = arm_current_el(env);
if (!pauth_key_enabled(env, el, SCTLR_EnIB)) {
return x;
}
-pauth_check_trap(env, el, GETPC());
-return pauth_auth(env, x, y, &env->keys.apib, false, 1);
+pauth_check_trap(env, el, ra);
+return pauth_auth(env, x, y, &env->keys.apib, false, 1, ra, is_combined);
}
-uint64_t HELPER(autda)(CPUARMState *env, uint64_t x, uint64_t y)
+uint64_t HELPER(autib)(CPUARMState *env, uint64_t x, uint64_t y)
+{
+return pauth_autib(env, x, y, GETPC(), false);
+}
+
+uint64_t HELPER(autib_combined)(CPUARMState *env, uint64_t x, uint64_t y)
+{
+return pauth_autib(env, x, y, GETPC(), true);
+}
+
+static uint64_t pauth_autda(CPUARMState *env, uint64_t x, uint64_t y,
+uintptr_t ra, bool is_combined)
{
int el = arm_current_el(env);
if (!pauth_key_enabled(env, el, SCTLR_EnDA)) {
return x;
}
-pauth_check_trap(env, el, GETPC());
-return pauth_auth(env, x, y, &env->keys.apda, true, 0);
+pauth_check_trap(env, el, ra);
+return pauth_auth(env, x, y, &env->keys.apda, true, 0, ra, is_combined);
}
-uint64_t HELPER(autdb)(CPUARMState *env, uint64_t x, uint64_t y)
+uint64_t HELPER(autda)(CPUARMState *env, uint64_t x, uint64_t y)
+{
+return pauth_autda(env, x, y, GETPC(), false);
+}
+
+uint64_t HELPER(autda_combined)(CPUARMState *env, uint64_t x, uint64_t y)
+{
+return pauth_autda(env, x, y, GETPC(), true);
+}
+
+static uint64_t pauth_autdb(CPUARMState *env, uint64_t x, uint64_t y,
+uintptr_t ra, bool is_combined)
{
i
[PULL v4 10/10] meson.build: enable xenpv machine build for ARM
From: Vikram Garhwal
Add CONFIG_XEN for aarch64 device to support build for ARM targets.
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Alex Bennée
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 481865bfa9..cfa98e9e25 100644
--- a/meson.build
+++ b/meson.build
@@ -136,7 +136,7 @@ endif
if cpu in ['x86', 'x86_64', 'arm', 'aarch64']
# i386 emulator provides xenpv machine type for multiple architectures
accelerator_targets += {
-'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu'],
+'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu'],
}
endif
if cpu in ['x86', 'x86_64']
--
2.25.1
[PULL v4 02/10] hw/i386/xen: rearrange xen_hvm_init_pc
From: Vikram Garhwal
In preparation to moving most of xen-hvm code to an arch-neutral location,
move non IOREQ references to:
- xen_get_vmport_regs_pfn
- xen_suspend_notifier
- xen_wakeup_notifier
- xen_ram_init
towards the end of the xen_hvm_init_pc() function.
This is done to keep the common ioreq functions in one place which will be
moved to new function in next patch in order to make it common to both x86 and
aarch64 machines.
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/i386/xen/xen-hvm.c | 49 ++-
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index ab8f1b61ee..7a7764240e 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1419,12 +1419,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
state->exit.notify = xen_exit_notifier;
qemu_add_exit_notifier(&state->exit);
-state->suspend.notify = xen_suspend_notifier;
-qemu_register_suspend_notifier(&state->suspend);
-
-state->wakeup.notify = xen_wakeup_notifier;
-qemu_register_wakeup_notifier(&state->wakeup);
-
/*
* Register wake-up support in QMP query-current-machine API
*/
@@ -1435,23 +1429,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
goto err;
}
-rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn);
-if (!rc) {
-DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
-state->shared_vmport_page =
-xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
- 1, &ioreq_pfn, NULL);
-if (state->shared_vmport_page == NULL) {
-error_report("map shared vmport IO page returned error %d
handle=%p",
- errno, xen_xc);
-goto err;
-}
-} else if (rc != -ENOSYS) {
-error_report("get vmport regs pfn returned error %d, rc=%d",
- errno, rc);
-goto err;
-}
-
/* Note: cpus is empty at this point in init */
state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus);
@@ -1490,7 +1467,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
#else
xen_map_cache_init(NULL, state);
#endif
-xen_ram_init(pcms, ms->ram_size, ram_memory);
qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state);
@@ -1511,6 +1487,31 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
QLIST_INIT(&xen_physmap);
xen_read_physmap(state);
+state->suspend.notify = xen_suspend_notifier;
+qemu_register_suspend_notifier(&state->suspend);
+
+state->wakeup.notify = xen_wakeup_notifier;
+qemu_register_wakeup_notifier(&state->wakeup);
+
+rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn);
+if (!rc) {
+DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
+state->shared_vmport_page =
+xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
+ 1, &ioreq_pfn, NULL);
+if (state->shared_vmport_page == NULL) {
+error_report("map shared vmport IO page returned error %d
handle=%p",
+ errno, xen_xc);
+goto err;
+}
+} else if (rc != -ENOSYS) {
+error_report("get vmport regs pfn returned error %d, rc=%d",
+ errno, rc);
+goto err;
+}
+
+xen_ram_init(pcms, ms->ram_size, ram_memory);
+
/* Disable ACPI build because Xen handles it */
pcms->acpi_build_enabled = false;
--
2.25.1
[PULL v4 06/10] hw/xen/xen-hvm-common: skip ioreq creation on ioreq registration failure
From: Stefano Stabellini
On ARM it is possible to have a functioning xenpv machine with only the
PV backends and no IOREQ server. If the IOREQ server creation fails continue
to the PV backends initialization.
Also, moved the IOREQ registration and mapping subroutine to new function
xen_do_ioreq_register().
Signed-off-by: Stefano Stabellini
Signed-off-by: Vikram Garhwal
Reviewed-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/xen/xen-hvm-common.c | 57 +++--
1 file changed, 38 insertions(+), 19 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index a31b067404..cb82f4b83d 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -764,27 +764,12 @@ void xen_shutdown_fatal_error(const char *fmt, ...)
qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_ERROR);
}
-void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
-MemoryListener xen_memory_listener)
+static void xen_do_ioreq_register(XenIOState *state,
+ unsigned int max_cpus,
+ MemoryListener xen_memory_listener)
{
int i, rc;
-setup_xen_backend_ops();
-
-state->xce_handle = qemu_xen_evtchn_open();
-if (state->xce_handle == NULL) {
-perror("xen: event channel open");
-goto err;
-}
-
-state->xenstore = xs_daemon_open();
-if (state->xenstore == NULL) {
-perror("xen: xenstore open");
-goto err;
-}
-
-xen_create_ioreq_server(xen_domid, &state->ioservid);
-
state->exit.notify = xen_exit_notifier;
qemu_add_exit_notifier(&state->exit);
@@ -849,12 +834,46 @@ void xen_register_ioreq(XenIOState *state, unsigned int
max_cpus,
QLIST_INIT(&state->dev_list);
device_listener_register(&state->device_listener);
+return;
+
+err:
+error_report("xen hardware virtual machine initialisation failed");
+exit(1);
+}
+
+void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
+MemoryListener xen_memory_listener)
+{
+int rc;
+
+setup_xen_backend_ops();
+
+state->xce_handle = qemu_xen_evtchn_open();
+if (state->xce_handle == NULL) {
+perror("xen: event channel open");
+goto err;
+}
+
+state->xenstore = xs_daemon_open();
+if (state->xenstore == NULL) {
+perror("xen: xenstore open");
+goto err;
+}
+
+rc = xen_create_ioreq_server(xen_domid, &state->ioservid);
+if (!rc) {
+xen_do_ioreq_register(state, max_cpus, xen_memory_listener);
+} else {
+warn_report("xen: failed to create ioreq server");
+}
+
xen_bus_init();
xen_be_init();
return;
+
err:
-error_report("xen hardware virtual machine initialisation failed");
+error_report("xen hardware virtual machine backend registration failed");
exit(1);
}
--
2.25.1
[PULL v4 05/10] include/hw/xen/xen_common: return error from xen_create_ioreq_server
From: Stefano Stabellini
From: Stefano Stabellini
This is done to prepare for enabling xenpv support for ARM architecture.
On ARM it is possible to have a functioning xenpv machine with only the
PV backends and no IOREQ server. If the IOREQ server creation fails,
continue to the PV backends initialization.
Signed-off-by: Stefano Stabellini
Signed-off-by: Vikram Garhwal
Reviewed-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
include/hw/xen/xen_native.h | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
index f11eb423e3..4dce905fde 100644
--- a/include/hw/xen/xen_native.h
+++ b/include/hw/xen/xen_native.h
@@ -463,8 +463,8 @@ static inline void xen_unmap_pcidev(domid_t dom,
PCI_FUNC(pci_dev->devfn));
}
-static inline void xen_create_ioreq_server(domid_t dom,
- ioservid_t *ioservid)
+static inline int xen_create_ioreq_server(domid_t dom,
+ ioservid_t *ioservid)
{
int rc = xendevicemodel_create_ioreq_server(xen_dmod, dom,
HVM_IOREQSRV_BUFIOREQ_ATOMIC,
@@ -472,12 +472,14 @@ static inline void xen_create_ioreq_server(domid_t dom,
if (rc == 0) {
trace_xen_ioreq_server_create(*ioservid);
-return;
+return rc;
}
*ioservid = 0;
use_default_ioreq_server = true;
trace_xen_default_ioreq_server();
+
+return rc;
}
static inline void xen_destroy_ioreq_server(domid_t dom,
--
2.25.1
[PULL v4 09/10] hw/arm: introduce xenpvh machine
From: Vikram Garhwal
Add a new machine xenpvh which creates a IOREQ server to register/connect with
Xen Hypervisor.
Optional: When CONFIG_TPM is enabled, it also creates a tpm-tis-device, adds a
TPM emulator and connects to swtpm running on host machine via chardev socket
and support TPM functionalities for a guest domain.
Extra command line for aarch64 xenpvh QEMU to connect to swtpm:
-chardev socket,id=chrtpm,path=/tmp/myvtpm2/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-machine tpm-base-addr=0x0c00 \
swtpm implements a TPM software emulator(TPM 1.2 & TPM 2) built on libtpms and
provides access to TPM functionality over socket, chardev and CUSE interface.
Github repo: https://github.com/stefanberger/swtpm
Example for starting swtpm on host machine:
mkdir /tmp/vtpm2
swtpm socket --tpmstate dir=/tmp/vtpm2 \
--ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock &
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Stefano Stabellini
---
docs/system/arm/xenpvh.rst| 34 +++
docs/system/target-arm.rst| 1 +
hw/arm/meson.build| 2 +
hw/arm/xen_arm.c | 181 ++
include/hw/arm/xen_arch_hvm.h | 9 ++
include/hw/xen/arch_hvm.h | 2 +
6 files changed, 229 insertions(+)
create mode 100644 docs/system/arm/xenpvh.rst
create mode 100644 hw/arm/xen_arm.c
create mode 100644 include/hw/arm/xen_arch_hvm.h
diff --git a/docs/system/arm/xenpvh.rst b/docs/system/arm/xenpvh.rst
new file mode 100644
index 00..e1655c7ab8
--- /dev/null
+++ b/docs/system/arm/xenpvh.rst
@@ -0,0 +1,34 @@
+XENPVH (``xenpvh``)
+=
+This machine creates a IOREQ server to register/connect with Xen Hypervisor.
+
+When TPM is enabled, this machine also creates a tpm-tis-device at a user input
+tpm base address, adds a TPM emulator and connects to a swtpm application
+running on host machine via chardev socket. This enables xenpvh to support TPM
+functionalities for a guest domain.
+
+More information about TPM use and installing swtpm linux application can be
+found at: docs/specs/tpm.rst.
+
+Example for starting swtpm on host machine:
+.. code-block:: console
+
+mkdir /tmp/vtpm2
+swtpm socket --tpmstate dir=/tmp/vtpm2 \
+--ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock &
+
+Sample QEMU xenpvh commands for running and connecting with Xen:
+.. code-block:: console
+
+qemu-system-aarch64 -xen-domid 1 \
+-chardev socket,id=libxl-cmd,path=qmp-libxl-1,server=on,wait=off \
+-mon chardev=libxl-cmd,mode=control \
+-chardev socket,id=libxenstat-cmd,path=qmp-libxenstat-1,server=on,wait=off
\
+-mon chardev=libxenstat-cmd,mode=control \
+-xen-attach -name guest0 -vnc none -display none -nographic \
+-machine xenpvh -m 1301 \
+-chardev socket,id=chrtpm,path=tmp/vtpm2/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm -machine tpm-base-addr=0x0C00
+
+In above QEMU command, last two lines are for connecting xenpvh QEMU to swtpm
+via chardev socket.
diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst
index a12b6bca05..790ac1b8a2 100644
--- a/docs/system/target-arm.rst
+++ b/docs/system/target-arm.rst
@@ -107,6 +107,7 @@ undocumented; you can get a complete list by running
arm/stm32
arm/virt
arm/xlnx-versal-virt
+ arm/xenpvh
Emulated CPU architecture support
=
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 870ec67376..4f94f821b0 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -63,6 +63,8 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true:
files('fsl-imx7.c', 'mcimx7d-sabre.
arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c',
'mcimx6ul-evk.c'))
arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
+arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c'))
+arm_ss.add_all(xen_ss)
softmmu_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
softmmu_ss.add(when: 'CONFIG_EXYNOS4', if_true: files('exynos4_boards.c'))
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
new file mode 100644
index 00..19b1cb81ad
--- /dev/null
+++ b/hw/arm/xen_arm.c
@@ -0,0 +1,181 @@
+/*
+ * QEMU ARM Xen PVH Machine
+ *
+ *
+ * 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 "A
[PULL v4 08/10] meson.build: do not set have_xen_pci_passthrough for aarch64 targets
From: Stefano Stabellini
have_xen_pci_passthrough is only used for Xen x86 VMs.
Signed-off-by: Stefano Stabellini
Reviewed-by: Alex Bennée
---
meson.build | 2 ++
1 file changed, 2 insertions(+)
diff --git a/meson.build b/meson.build
index 34306a6205..481865bfa9 100644
--- a/meson.build
+++ b/meson.build
@@ -1726,6 +1726,8 @@ have_xen_pci_passthrough =
get_option('xen_pci_passthrough') \
error_message: 'Xen PCI passthrough requested but Xen not enabled')
\
.require(targetos == 'linux',
error_message: 'Xen PCI passthrough not available on this
platform') \
+ .require(cpu == 'x86' or cpu == 'x86_64',
+ error_message: 'Xen PCI passthrough not available on this
platform') \
.allowed()
--
2.25.1
[PULL v4 07/10] hw/xen/xen-hvm-common: Use g_new and error_report
From: Vikram Garhwal
Replace g_malloc with g_new and perror with error_report.
Signed-off-by: Vikram Garhwal
Reviewed-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/xen/xen-hvm-common.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index cb82f4b83d..42339c96bd 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -33,7 +33,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
MemoryRegion *mr,
trace_xen_ram_alloc(ram_addr, size);
nr_pfn = size >> TARGET_PAGE_BITS;
-pfn_list = g_malloc(sizeof (*pfn_list) * nr_pfn);
+pfn_list = g_new(xen_pfn_t, nr_pfn);
for (i = 0; i < nr_pfn; i++) {
pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i;
@@ -730,7 +730,7 @@ void destroy_hvm_domain(bool reboot)
return;
}
if (errno != ENOTTY /* old Xen */) {
-perror("xendevicemodel_shutdown failed");
+error_report("xendevicemodel_shutdown failed with error %d",
errno);
}
/* well, try the old thing then */
}
@@ -784,7 +784,7 @@ static void xen_do_ioreq_register(XenIOState *state,
}
/* Note: cpus is empty at this point in init */
-state->cpu_by_vcpu_id = g_malloc0(max_cpus * sizeof(CPUState *));
+state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus);
rc = xen_set_ioreq_server_state(xen_domid, state->ioservid, true);
if (rc < 0) {
@@ -793,7 +793,7 @@ static void xen_do_ioreq_register(XenIOState *state,
goto err;
}
-state->ioreq_local_port = g_malloc0(max_cpus * sizeof (evtchn_port_t));
+state->ioreq_local_port = g_new0(evtchn_port_t, max_cpus);
/* FIXME: how about if we overflow the page here? */
for (i = 0; i < max_cpus; i++) {
@@ -850,13 +850,13 @@ void xen_register_ioreq(XenIOState *state, unsigned int
max_cpus,
state->xce_handle = qemu_xen_evtchn_open();
if (state->xce_handle == NULL) {
-perror("xen: event channel open");
+error_report("xen: event channel open failed with error %d", errno);
goto err;
}
state->xenstore = xs_daemon_open();
if (state->xenstore == NULL) {
-perror("xen: xenstore open");
+error_report("xen: xenstore open failed with error %d", errno);
goto err;
}
--
2.25.1
[PULL v4 04/10] xen-hvm: reorganize xen-hvm and move common function to xen-hvm-common
From: Stefano Stabellini This patch does following: 1. creates arch_handle_ioreq() and arch_xen_set_memory(). This is done in preparation for moving most of xen-hvm code to an arch-neutral location, move the x86-specific portion of xen_set_memory to arch_xen_set_memory. Also, move handle_vmport_ioreq to arch_handle_ioreq. 2. Pure code movement: move common functions to hw/xen/xen-hvm-common.c Extract common functionalities from hw/i386/xen/xen-hvm.c and move them to hw/xen/xen-hvm-common.c. These common functions are useful for creating an IOREQ server. xen_hvm_init_pc() contains the architecture independent code for creating and mapping a IOREQ server, connecting memory and IO listeners, initializing a xen bus and registering backends. Moved this common xen code to a new function xen_register_ioreq() which can be used by both x86 and ARM machines. Following functions are moved to hw/xen/xen-hvm-common.c: xen_vcpu_eport(), xen_vcpu_ioreq(), xen_ram_alloc(), xen_set_memory(), xen_region_add(), xen_region_del(), xen_io_add(), xen_io_del(), xen_device_realize(), xen_device_unrealize(), cpu_get_ioreq_from_shared_memory(), cpu_get_ioreq(), do_inp(), do_outp(), rw_phys_req_item(), read_phys_req_item(), write_phys_req_item(), cpu_ioreq_pio(), cpu_ioreq_move(), cpu_ioreq_config(), handle_ioreq(), handle_buffered_iopage(), handle_buffered_io(), cpu_handle_ioreq(), xen_main_loop_prepare(), xen_hvm_change_state_handler(), xen_exit_notifier(), xen_map_ioreq_server(), destroy_hvm_domain() and xen_shutdown_fatal_error() 3. Removed static type from below functions: 1. xen_region_add() 2. xen_region_del() 3. xen_io_add() 4. xen_io_del() 5. xen_device_realize() 6. xen_device_unrealize() 7. xen_hvm_change_state_handler() 8. cpu_ioreq_pio() 9. xen_exit_notifier() 4. Replace TARGET_PAGE_SIZE with XC_PAGE_SIZE to match the page side with Xen. Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Acked-by: Stefano Stabellini --- hw/i386/xen/trace-events| 14 - hw/i386/xen/xen-hvm.c | 1016 ++- hw/xen/meson.build |5 +- hw/xen/trace-events | 14 + hw/xen/xen-hvm-common.c | 860 ++ include/hw/i386/xen_arch_hvm.h | 11 + include/hw/xen/arch_hvm.h |3 + include/hw/xen/xen-hvm-common.h | 99 +++ 8 files changed, 1054 insertions(+), 968 deletions(-) create mode 100644 hw/xen/xen-hvm-common.c create mode 100644 include/hw/i386/xen_arch_hvm.h create mode 100644 include/hw/xen/arch_hvm.h create mode 100644 include/hw/xen/xen-hvm-common.h diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events index a0c89d91c4..5d0a8d6dcf 100644 --- a/hw/i386/xen/trace-events +++ b/hw/i386/xen/trace-events @@ -7,17 +7,3 @@ xen_platform_log(char *s) "xen platform: %s" xen_pv_mmio_read(uint64_t addr) "WARNING: read from Xen PV Device MMIO space (address 0x%"PRIx64")" xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (address 0x%"PRIx64")" -# xen-hvm.c -xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: 0x%lx, size 0x%lx" -xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "0x%"PRIx64" size 0x%lx, log_dirty %i" -handle_ioreq(void *req, uint32_t type, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p type=%d dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -handle_ioreq_read(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p read type=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -handle_ioreq_write(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p write type=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -cpu_ioreq_pio(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p pio dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -cpu_ioreq_pio_read_reg(void *req, uint64_t data, uint64_t addr, uint32_t size) "I/O=%p pio read reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d" -cpu_ioreq_pio_write_reg(void *req, uint64_t data, uint64_t addr, uint32_t size) "I/O=%p pio write reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d" -cpu_ioreq_move(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p copy dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -xen_map_resource_ioreq(uint32_t id, void *addr) "id: %u addr: %p" -cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg,
[PULL v4 01/10] hw/i386/xen/: move xen-mapcache.c to hw/xen/
From: Vikram Garhwal
xen-mapcache.c contains common functions which can be used for enabling Xen on
aarch64 with IOREQ handling. Moving it out from hw/i386/xen to hw/xen to make it
accessible for both aarch64 and x86.
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/i386/meson.build | 1 +
hw/i386/xen/meson.build | 1 -
hw/i386/xen/trace-events | 5 -
hw/xen/meson.build | 4
hw/xen/trace-events | 5 +
hw/{i386 => }/xen/xen-mapcache.c | 0
6 files changed, 10 insertions(+), 6 deletions(-)
rename hw/{i386 => }/xen/xen-mapcache.c (100%)
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 213e2e82b3..cfdbfdcbcb 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -33,5 +33,6 @@ subdir('kvm')
subdir('xen')
i386_ss.add_all(xenpv_ss)
+i386_ss.add_all(xen_ss)
hw_arch += {'i386': i386_ss}
diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index 2e64a34e16..3dc4c4f106 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -1,6 +1,5 @@
i386_ss.add(when: 'CONFIG_XEN', if_true: files(
'xen-hvm.c',
- 'xen-mapcache.c',
'xen_apic.c',
'xen_pvdevice.c',
))
diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events
index 5d6be61090..a0c89d91c4 100644
--- a/hw/i386/xen/trace-events
+++ b/hw/i386/xen/trace-events
@@ -21,8 +21,3 @@ xen_map_resource_ioreq(uint32_t id, void *addr) "id: %u addr:
%p"
cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size,
uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t size,
uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
-# xen-mapcache.c
-xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64
-xen_remap_bucket(uint64_t index) "index 0x%"PRIx64
-xen_map_cache_return(void* ptr) "%p"
-
diff --git a/hw/xen/meson.build b/hw/xen/meson.build
index 19c6aabc7c..202752e557 100644
--- a/hw/xen/meson.build
+++ b/hw/xen/meson.build
@@ -26,3 +26,7 @@ else
endif
specific_ss.add_all(when: ['CONFIG_XEN', xen], if_true: xen_specific_ss)
+
+xen_ss = ss.source_set()
+
+xen_ss.add(when: 'CONFIG_XEN', if_true: files('xen-mapcache.c'))
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index 55c9e1df68..f977c7c8c6 100644
--- a/hw/xen/trace-events
+++ b/hw/xen/trace-events
@@ -41,3 +41,8 @@ xs_node_vprintf(char *path, char *value) "%s %s"
xs_node_vscanf(char *path, char *value) "%s %s"
xs_node_watch(char *path) "%s"
xs_node_unwatch(char *path) "%s"
+
+# xen-mapcache.c
+xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64
+xen_remap_bucket(uint64_t index) "index 0x%"PRIx64
+xen_map_cache_return(void* ptr) "%p"
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
similarity index 100%
rename from hw/i386/xen/xen-mapcache.c
rename to hw/xen/xen-mapcache.c
--
2.25.1
[PULL v4 03/10] hw/i386/xen/xen-hvm: move x86-specific fields out of XenIOState
From: Stefano Stabellini
In preparation to moving most of xen-hvm code to an arch-neutral location, move:
- shared_vmport_page
- log_for_dirtybit
- dirty_bitmap
- suspend
- wakeup
out of XenIOState struct as these are only used on x86, especially the ones
related to dirty logging.
Updated XenIOState can be used for both aarch64 and x86.
Also, remove free_phys_offset as it was unused.
Signed-off-by: Stefano Stabellini
Signed-off-by: Vikram Garhwal
Reviewed-by: Paul Durrant
Reviewed-by: Alex Bennée
---
hw/i386/xen/xen-hvm.c | 58 ---
1 file changed, 27 insertions(+), 31 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7a7764240e..01bf947f1c 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -74,6 +74,7 @@ struct shared_vmport_iopage {
};
typedef struct shared_vmport_iopage shared_vmport_iopage_t;
#endif
+static shared_vmport_iopage_t *shared_vmport_page;
static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
{
@@ -96,6 +97,11 @@ typedef struct XenPhysmap {
} XenPhysmap;
static QLIST_HEAD(, XenPhysmap) xen_physmap;
+static const XenPhysmap *log_for_dirtybit;
+/* Buffer used by xen_sync_dirty_bitmap */
+static unsigned long *dirty_bitmap;
+static Notifier suspend;
+static Notifier wakeup;
typedef struct XenPciDevice {
PCIDevice *pci_dev;
@@ -106,7 +112,6 @@ typedef struct XenPciDevice {
typedef struct XenIOState {
ioservid_t ioservid;
shared_iopage_t *shared_page;
-shared_vmport_iopage_t *shared_vmport_page;
buffered_iopage_t *buffered_io_page;
xenforeignmemory_resource_handle *fres;
QEMUTimer *buffered_io_timer;
@@ -126,14 +131,8 @@ typedef struct XenIOState {
MemoryListener io_listener;
QLIST_HEAD(, XenPciDevice) dev_list;
DeviceListener device_listener;
-hwaddr free_phys_offset;
-const XenPhysmap *log_for_dirtybit;
-/* Buffer used by xen_sync_dirty_bitmap */
-unsigned long *dirty_bitmap;
Notifier exit;
-Notifier suspend;
-Notifier wakeup;
} XenIOState;
/* Xen specific function for piix pci */
@@ -463,10 +462,10 @@ static int xen_remove_from_physmap(XenIOState *state,
}
QLIST_REMOVE(physmap, list);
-if (state->log_for_dirtybit == physmap) {
-state->log_for_dirtybit = NULL;
-g_free(state->dirty_bitmap);
-state->dirty_bitmap = NULL;
+if (log_for_dirtybit == physmap) {
+log_for_dirtybit = NULL;
+g_free(dirty_bitmap);
+dirty_bitmap = NULL;
}
g_free(physmap);
@@ -627,16 +626,16 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
return;
}
-if (state->log_for_dirtybit == NULL) {
-state->log_for_dirtybit = physmap;
-state->dirty_bitmap = g_new(unsigned long, bitmap_size);
-} else if (state->log_for_dirtybit != physmap) {
+if (log_for_dirtybit == NULL) {
+log_for_dirtybit = physmap;
+dirty_bitmap = g_new(unsigned long, bitmap_size);
+} else if (log_for_dirtybit != physmap) {
/* Only one range for dirty bitmap can be tracked. */
return;
}
rc = xen_track_dirty_vram(xen_domid, start_addr >> TARGET_PAGE_BITS,
- npages, state->dirty_bitmap);
+ npages, dirty_bitmap);
if (rc < 0) {
#ifndef ENODATA
#define ENODATA ENOENT
@@ -651,7 +650,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
}
for (i = 0; i < bitmap_size; i++) {
-unsigned long map = state->dirty_bitmap[i];
+unsigned long map = dirty_bitmap[i];
while (map != 0) {
j = ctzl(map);
map &= ~(1ul << j);
@@ -677,12 +676,10 @@ static void xen_log_start(MemoryListener *listener,
static void xen_log_stop(MemoryListener *listener, MemoryRegionSection
*section,
int old, int new)
{
-XenIOState *state = container_of(listener, XenIOState, memory_listener);
-
if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
-state->log_for_dirtybit = NULL;
-g_free(state->dirty_bitmap);
-state->dirty_bitmap = NULL;
+log_for_dirtybit = NULL;
+g_free(dirty_bitmap);
+dirty_bitmap = NULL;
/* Disable dirty bit tracking */
xen_track_dirty_vram(xen_domid, 0, 0, NULL);
}
@@ -1022,9 +1019,9 @@ static void handle_vmport_ioreq(XenIOState *state,
ioreq_t *req)
{
vmware_regs_t *vmport_regs;
-assert(state->shared_vmport_page);
+assert(shared_vmport_page);
vmport_regs =
-&state->shared_vmport_page->vcpu_vmport_regs[state->send_vcpu];
+&shared_vmport_page->vcpu_vmport_regs[state->send_vcpu];
QEMU_BUILD_BUG_ON(sizeof(*req) < sizeof(*vmport_regs));
current_cpu = state->cpu_by_vcpu_id[state->send_vcpu];
@@ -1472,7 +1469,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
state->memory_listener = xen_memory_listener;
[PULL v4 0/10] xenpvh4-tag
Hi Peter,
Vikram fixed the gitlab test problem again. This time I am providing
proof that it is not introducing build regressions:
baseline: https://gitlab.com/sstabellini/qemu/-/pipelines/894578994
with new commits: https://gitlab.com/sstabellini/qemu/-/pipelines/894471770
Cheers,
Stefano
The following changes since commit 5f9dd6a8ce3961db4ce47411ed2097ad88bdf5fc:
Merge tag 'pull-9p-20230608' of https://github.com/cschoenebeck/qemu into
staging (2023-06-08 08:47:35 -0700)
are available in the Git repository at:
https://gitlab.com/sstabellini/qemu xenpvh4-tag
for you to fetch changes up to 236c27fd61dd0961631076a56286dd108fe5ef1f:
meson.build: enable xenpv machine build for ARM (2023-06-08 16:54:31 -0700)
Stefano Stabellini (5):
hw/i386/xen/xen-hvm: move x86-specific fields out of XenIOState
xen-hvm: reorganize xen-hvm and move common function to xen-hvm-common
include/hw/xen/xen_common: return error from xen_create_ioreq_server
hw/xen/xen-hvm-common: skip ioreq creation on ioreq registration failure
meson.build: do not set have_xen_pci_passthrough for aarch64 targets
Vikram Garhwal (5):
hw/i386/xen/: move xen-mapcache.c to hw/xen/
hw/i386/xen: rearrange xen_hvm_init_pc
hw/xen/xen-hvm-common: Use g_new and error_report
hw/arm: introduce xenpvh machine
meson.build: enable xenpv machine build for ARM
docs/system/arm/xenpvh.rst | 34 ++
docs/system/target-arm.rst |1 +
hw/arm/meson.build |2 +
hw/arm/xen_arm.c | 181 +++
hw/i386/meson.build |1 +
hw/i386/xen/meson.build |1 -
hw/i386/xen/trace-events | 19 -
hw/i386/xen/xen-hvm.c| 1075 --
hw/xen/meson.build |7 +
hw/xen/trace-events | 19 +
hw/xen/xen-hvm-common.c | 879 +++
hw/{i386 => }/xen/xen-mapcache.c |0
include/hw/arm/xen_arch_hvm.h|9 +
include/hw/i386/xen_arch_hvm.h | 11 +
include/hw/xen/arch_hvm.h|5 +
include/hw/xen/xen-hvm-common.h | 99
include/hw/xen/xen_native.h |8 +-
meson.build |4 +-
18 files changed, 1347 insertions(+), 1008 deletions(-)
create mode 100644 docs/system/arm/xenpvh.rst
create mode 100644 hw/arm/xen_arm.c
create mode 100644 hw/xen/xen-hvm-common.c
rename hw/{i386 => }/xen/xen-mapcache.c (100%)
create mode 100644 include/hw/arm/xen_arch_hvm.h
create mode 100644 include/hw/i386/xen_arch_hvm.h
create mode 100644 include/hw/xen/arch_hvm.h
create mode 100644 include/hw/xen/xen-hvm-common.h
Re: [PATCH v2] linux-user: Return EINVAL for getgroups() with negative gidsetsize
09.06.2023 19:29, Peter Maydell wrote: Coverity doesn't like the way we might end up calling getgroups() with a NULL grouplist pointer. This is fine for the special case of gidsetsize == 0, but we will also do it if the guest passes us a negative gidsetsize. (CID 1512465) Explicitly fail the negative gidsetsize with EINVAL, as the kernel does. This means we definitely only call the libc getgroups() with valid parameters. It also brings the getgroups() code in to line with the setgroups() code. Possibly Coverity may still complain about getgroups(0, NULL), but that would be a false positive. Signed-off-by: Peter Maydell --- v2: also change TARGET_NR_getgroups32 code Reviewed-by: Michael Tokarev This can be applied to -trivial just fine, I think. There's another change in there already in exactly this area. Thank you Peter! /mjt
[PATCH] hw/i386/pc: Clean up pc_machine_initfn
To use the newly introduced PC machine class local variable. Suggedted-by: Igor Mammedov Signed-off-by: Suravee Suthikulpanit --- hw/i386/pc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 33ffb03a32..f8d105e829 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1864,7 +1864,7 @@ static void pc_machine_initfn(Object *obj) pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type; /* acpi build is enabled by default if machine supports it */ -pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build; +pcms->acpi_build_enabled = pcmc->has_acpi_build; pcms->smbus_enabled = true; pcms->sata_enabled = true; pcms->i8042_enabled = true; -- 2.34.1
Re: [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
On 6/8/2023 3:40 PM, Igor Mammedov wrote:
On Wed, 7 Jun 2023 15:57:16 -0500
Suravee Suthikulpanit wrote:
Currently, pc-q35 and pc-i44fx machine models are default to use SMBIOS 2.8
(32-bit entry point). Since SMBIOS 3.0 (64-bit entry point) is now fully
supported since QEMU 7.0, default to use SMBIOS 3.0 for newer machine
models. This is necessary to avoid the following message when launching
a VM with large number of vcpus.
"SMBIOS 2.1 table length 66822 exceeds 65535"
Signed-off-by: Suravee Suthikulpanit
Looks good to me (see comment below for extra cleanup possibility):
Reviewed-by: Igor Mammedov
---
hw/i386/pc.c | 4 +++-
hw/i386/pc_piix.c| 5 +
hw/i386/pc_q35.c | 5 +
include/hw/i386/pc.h | 1 +
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index bb62c994fa..33ffb03a32 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1853,6 +1853,7 @@ static void pc_machine_set_max_fw_size(Object *obj,
Visitor *v,
static void pc_machine_initfn(Object *obj)
{
PCMachineState *pcms = PC_MACHINE(obj);
+PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
since you introduce this local,
I suggest you to post an extra clean up patch on top of this series
here is a line to cleanup with 'pcmc'
/* acpi build is enabled by default if machine supports it */
pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
Sure just sent out the "[PATCH] hw/i386/pc: Clean up pc_machine_initfn".
Thanks,
Suravee
[PATCH v2] linux-user: Return EINVAL for getgroups() with negative gidsetsize
Coverity doesn't like the way we might end up calling getgroups()
with a NULL grouplist pointer. This is fine for the special case
of gidsetsize == 0, but we will also do it if the guest passes
us a negative gidsetsize. (CID 1512465)
Explicitly fail the negative gidsetsize with EINVAL, as the kernel
does. This means we definitely only call the libc getgroups()
with valid parameters. It also brings the getgroups() code in
to line with the setgroups() code.
Possibly Coverity may still complain about getgroups(0, NULL), but
that would be a false positive.
Signed-off-by: Peter Maydell
---
v2: also change TARGET_NR_getgroups32 code
---
linux-user/syscall.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 83685f0aa59..bb29444ca7e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11676,7 +11676,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
g_autofree gid_t *grouplist = NULL;
int i;
-if (gidsetsize > NGROUPS_MAX) {
+if (gidsetsize > NGROUPS_MAX || gidsetsize < 0) {
return -TARGET_EINVAL;
}
if (gidsetsize > 0) {
@@ -12012,7 +12012,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
g_autofree gid_t *grouplist = NULL;
int i;
-if (gidsetsize > NGROUPS_MAX) {
+if (gidsetsize > NGROUPS_MAX || gidsetsize < 0) {
return -TARGET_EINVAL;
}
if (gidsetsize > 0) {
--
2.34.1
Re: arm: couple of finalize_memop related oddities
On Fri, 9 Jun 2023 at 17:03, Richard Henderson wrote: > > On 6/9/23 03:47, Peter Maydell wrote: > > I noticed a couple of finalize_memop related oddities while I > > was rebasing my decodetree series: > > > > (1) in disas_ldst_reg_imm9(), we calculate a memop, but then > > when we call gen_mte_check1_mmuidx() we don't pass the memop > > as that function's memop argument, we just pass size. Everywhere > > else that calls gen_mte_check* functions passes memop. Intentional? > > No, looks like a bug. > > > (2) disas_ldst_reg_roffset() and disas_ldst_reg_unsigned_imm() > > use finalize_memop() for both vector and normal register > > loads/stores. Should they be using finalize_memop_asimd() > > for the vector versions? > > > > (3) disas_ldst_multiple_struct() and disas_ldst_single_struct() > > use finalize_memop() even though they always load/store > > vector registers. Should they be using finalize_memop_asimd() ? > > Yes. > > Everywhere that uses CreateAccDescASIMD in the pseudocode should use > finalize_memop_asimd > for clarity. (If size is never 128-bit then it will be same as just > finalize_memop, so > some of those places were not actual bugs. But definitely unclear.) OK, thanks. The rebase of my decodetree patches for ld/st turned out hairy enough that I think I'll resend a v2 of that, so I'll add in some patches at the start that fix these finalize_memop bits before the conversion. -- PMM
Re: arm: couple of finalize_memop related oddities
On 6/9/23 03:47, Peter Maydell wrote: I noticed a couple of finalize_memop related oddities while I was rebasing my decodetree series: (1) in disas_ldst_reg_imm9(), we calculate a memop, but then when we call gen_mte_check1_mmuidx() we don't pass the memop as that function's memop argument, we just pass size. Everywhere else that calls gen_mte_check* functions passes memop. Intentional? No, looks like a bug. (2) disas_ldst_reg_roffset() and disas_ldst_reg_unsigned_imm() use finalize_memop() for both vector and normal register loads/stores. Should they be using finalize_memop_asimd() for the vector versions? (3) disas_ldst_multiple_struct() and disas_ldst_single_struct() use finalize_memop() even though they always load/store vector registers. Should they be using finalize_memop_asimd() ? Yes. Everywhere that uses CreateAccDescASIMD in the pseudocode should use finalize_memop_asimd for clarity. (If size is never 128-bit then it will be same as just finalize_memop, so some of those places were not actual bugs. But definitely unclear.) r~
Re: [PATCH v4 0/1] ROM migration
This is tagged, pull request will be delayed due to kvm forum though. On Fri, Jun 09, 2023 at 04:32:40PM +0300, Vladimir Sementsov-Ogievskiy wrote: > ping. > > On 22.05.23 23:17, Vladimir Sementsov-Ogievskiy wrote: > > v4: > > preparation patches are already merged to master > > 01: fix false-positive "error: ‘size’ may be used uninitialized", > > keep r-bs > > > > Vladimir Sementsov-Ogievskiy (1): > >pci: ROM preallocation for incoming migration > > > > hw/pci/pci.c | 79 ++-- > > 1 file changed, 46 insertions(+), 33 deletions(-) > > > > -- > Best regards, > Vladimir
[PATCH] target/arm: Return correct result for LDG when ATA=0
The LDG instruction loads the tag from a memory address (identified
by [Xn + offset]), and then merges that tag into the destination
register Xt. We implemented this correctly for the case when
allocation tags are enabled, but didn't get it right when ATA=0:
instead of merging the tag bits into Xt, we merged them into the
memory address [Xn + offset] and then set Xt to that.
Merge the tag bits into the old Xt value, as they should be.
Cc: qemu-sta...@nongnu.org
Fixes: c15294c1e36a7dd9b25 ("target/arm: Implement LDG, STG, ST2G instructions")
Signed-off-by: Peter Maydell
---
Spotted this while I was doing decodetree conversion stuff.
I guess nobody runs with MTE available but not turned on :-)
---
target/arm/tcg/translate-a64.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index aa93f37e216..9ead91a450b 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -4187,9 +4187,13 @@ static void disas_ldst_tag(DisasContext *s, uint32_t
insn)
if (s->ata) {
gen_helper_ldg(tcg_rt, cpu_env, addr, tcg_rt);
} else {
+/*
+ * Tag access disabled: we must check for aborts on the load
+ * load from [rn+offset], and then insert a 0 tag into rt.
+ */
clean_addr = clean_data_tbi(s, addr);
gen_probe_access(s, clean_addr, MMU_DATA_LOAD, MO_8);
-gen_address_with_allocation_tag0(tcg_rt, addr);
+gen_address_with_allocation_tag0(tcg_rt, tcg_rt);
}
} else {
tcg_rt = cpu_reg_sp(s, rt);
--
2.34.1
Re: [PATCH v14 08/10] Adding info [tb-list|tb] commands to HMP (WIP)
On Fri, 9 Jun 2023 at 15:32, Wu, Fei wrote: > > On 6/8/2023 5:23 PM, Peter Maydell wrote: > > On Thu, 8 Jun 2023 at 08:44, Wu, Fei wrote: > >> Is there any existing function to convert ram_addr_t to guest pa? > > > > Such a function would not be well-defined, because a block of RAM > > as specified by a ram_addr_t could be present at (aliased to) multiple > > guest physical addresses, or even currently not mapped to any guest > > physical address at all. And it could be present at different physical > > addresses for different vCPUs. > > > Thank you, Peter. What's the scenario of the last different physical > addresses for different vCPUs? You can have a board with two different CPUs, where one of them has a RAM MemoryRegion that it puts at address A in its address space, and the other puts it at address B. This is most likely with 'heterogenous' configurations where you have an application processor A and a board-support processor B. (I don't know if we actually have any board models like that currently, but it's logically possible.) > For this specific case, I found I don't have to convert ram_addr_t to > gpa, the real requirement is converting ram_addr_t to hva, this one > seems well defined using qemu_map_ram_ptr(0, ram_addr) ? That does work for ram_addr_t to hva, but but note the warning on the comment above that function: you need to be in an RCU critical section to avoid some other thread coming along and de-allocating the RAM under your feet. (Also it's tempting to remove that support for passing in 0 as the RAMBlock, because every other use in the codebase seems to pass in a RAMBlock pointer.) thanks -- PMM
[PATCH v3 0/8] misc AHCI cleanups
From: Niklas Cassel Hello John, Here comes some misc AHCI cleanups. Most are related to error handling. Please review. Changes since v2: -Squashed in the test commits that were sent out as a separate series into the patch "hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set", and reordered some of the patches, such that each and every commit passes the ahci test suite as a separate unit. This way it will be possible to perform a git bisect without seeing any failures in the ahci test suite. Kind regards, Niklas Niklas Cassel (8): hw/ide/ahci: remove stray backslash hw/ide/core: set ERR_STAT in unsupported command completion hw/ide/ahci: write D2H FIS when processing NCQ command hw/ide/ahci: simplify and document PxCI handling hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set hw/ide/ahci: fix ahci_write_fis_sdb() hw/ide/ahci: fix broken SError handling hw/ide/ahci.c | 112 +++--- hw/ide/core.c | 2 +- tests/qtest/libqos/ahci.c | 106 +++- tests/qtest/libqos/ahci.h | 8 +-- 4 files changed, 164 insertions(+), 64 deletions(-) -- 2.40.1
Re: [PATCH 0/4] hw/nvme: tp4146 misc
On Wed, May 24, 2023 at 01:19:00PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> A set of fixes and small quality-of-life improvements for the TP4146
> ("Flexible Data Placement") support.
>
> Klaus Jensen (4):
> hw/nvme: fix verification of number of ruhis
> hw/nvme: verify uniqueness of reclaim unit handle identifiers
> hw/nvme: add placement handle list ranges
> docs: update hw/nvme documentation for TP4146
>
> docs/system/devices/nvme.rst | 37 +++-
> hw/nvme/ns.c | 55
> hw/nvme/subsys.c | 6 ++--
> 3 files changed, 84 insertions(+), 14 deletions(-)
>
> --
> 2.40.0
>
>
Looks good to me :)
Reviewed-by: Jesper Wendel Devantier
[PATCH] 9pfs: deprecate 'proxy' backend
As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in
bad shape. Using the 'proxy' backend was already discouraged for safety
reasons before and we recommended to use the 'local' backend instead,
but now it is time to officially deprecate the 'proxy' backend.
Signed-off-by: Christian Schoenebeck
---
MAINTAINERS | 7 +++
docs/about/deprecated.rst | 17 +
fsdev/qemu-fsdev.c| 5 +
meson.build | 2 +-
softmmu/vl.c | 5 +
5 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 436b3f0afe..185d694b2e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2118,13 +2118,20 @@ S: Odd Fixes
W: https://wiki.qemu.org/Documentation/9p
F: hw/9pfs/
X: hw/9pfs/xen-9p*
+X: hw/9pfs/9p-proxy*
F: fsdev/
+X: fsdev/virtfs-proxy-helper.c
F: docs/tools/virtfs-proxy-helper.rst
F: tests/qtest/virtio-9p-test.c
F: tests/qtest/libqos/virtio-9p*
T: git https://gitlab.com/gkurz/qemu.git 9p-next
T: git https://github.com/cschoenebeck/qemu.git 9p.next
+virtio-9p-proxy
+F: hw/9pfs/9p-proxy*
+F: fsdev/virtfs-proxy-helper.c
+S: Obsolete
+
virtio-blk
M: Stefan Hajnoczi
L: qemu-bl...@nongnu.org
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 0743459862..9b2c780365 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -343,6 +343,23 @@ the addition of volatile memory support, it is now
necessary to distinguish
between persistent and volatile memory backends. As such, memdev is deprecated
in favor of persistent-memdev.
+``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1)
+^^
+
+The 9p ``proxy`` filesystem backend driver has been deprecated and will be
+removed in a future version of QEMU. Please use ``-fsdev local`` or
+``-virtfs local`` for using the ``local`` 9p filesystem backend instead.
+
+The 9p ``proxy`` backend was originally developed as an alternative to the 9p
+``local`` backend. The idea was to enhance security by dispatching actual low
+level filesystem operations from 9p server (QEMU process) over to a separate
+process (the virtfs-proxy-helper binary). However this alternative never gained
+momentum. The proxy backend is much slower than the local backend, hasn't seen
+any development in years, and showed to be less secure, especially due to the
+fact that its helper daemon must be run as root, whereas with the local backend
+QEMU is typically run as unprivileged user and allows to tighten behaviour by
+mapping permissions et al.
+
Block device options
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 3da64e9f72..242f54ab49 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp)
}
if (fsdriver) {
+if (strncmp(fsdriver, "proxy", 5) == 0) {
+warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' "
+"instead");
+}
+
for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) {
if (strcmp(FsDrivers[i].name, fsdriver) == 0) {
break;
diff --git a/meson.build b/meson.build
index 34306a6205..05c01b72bb 100644
--- a/meson.build
+++ b/meson.build
@@ -4170,7 +4170,7 @@ if have_block
summary_info += {'Block whitelist (ro)':
get_option('block_drv_ro_whitelist')}
summary_info += {'Use block whitelist in tools':
get_option('block_drv_whitelist_in_tools')}
summary_info += {'VirtFS (9P) support':have_virtfs}
- summary_info += {'VirtFS (9P) Proxy Helper support':
have_virtfs_proxy_helper}
+ summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)':
have_virtfs_proxy_helper}
summary_info += {'Live block migration':
config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')}
summary_info += {'replication support':
config_host_data.get('CONFIG_REPLICATION')}
summary_info += {'bochs support': get_option('bochs').allowed()}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..e60648b591 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3102,6 +3102,11 @@ void qemu_init(int argc, char **argv)
error_report("Usage: -virtfs fsdriver,mount_tag=tag");
exit(1);
}
+if (strncmp(qemu_opt_get(opts, "fsdriver"), "proxy", 5) == 0) {
+warn_report("'-virtfs proxy' is deprecated, use "
+"'-virtfs local' instead");
+}
+
fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
qemu_opts_id(opts) ?:
qemu_opt_get(opts, "mount_tag"),
--
2.30.2
Re: [EXT] Re: Concept of LD-ID in QEMU
On Thu, 8 Jun 2023 23:38:34 + Shesha Bhushan Sreenivasamurthy wrote: > Hi, > > Thinking a bit more, LD in CXL are PCIe endpoint functions. Therefore 1-1 > mapping of cxl-i2c device per PCIe device is sufficient, and we use function > number in BDF as the LD-ID. Does it makes sense ? LDs are PCIe endpoint functions (always function 0) as seen from the Virtual Heirarchies (as they end up under a particular vPBB - which look like a downstream port of a switch to the host) but they aren't from a more general topology point of view of actual fabric topology and when we are tunneling we address them via physical port, not virtual port I think (not read that bit of the spec for a while). See figure 7-23 in CXL 3.0 Outer tunneling command targets a port number (unwrapped at the switch), inner one targets the LD - unwrapped at the MLD and sent to appropriate LD including FM owned LD (if I understand this stack directly). Also no relationship between BDF and LD-ID so don't do that as the maximum ID is only 16 which would rather limit your PCI toplogies if that's the BDF as well. For now just do what you originally said and add an ID (starting from 0). We can probably do that automatically once more infrastructure is in place. Jonathan > > From: Jonathan Cameron > Sent: Thursday, June 8, 2023 3:36 AM > To: Shesha Bhushan Sreenivasamurthy > Cc: linux-...@vger.kernel.org ; > <"qemu-devel@nongnu.org" > Subject: [EXT] Re: Concept of LD-ID in QEMU > > External Email > > -- > > Shesha, > > You've sent an email with the 'In-reply-to' set to one of Terry's patches. > Please check why that happened and make sure you don't do that in future as > it hides your unrelated thread in email clients and the archives! > > See > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_linux-2Dcxl_20230607221651.2454764-2D1-2Dterry.bowman-40amd.com_T_-23t&d=DwIFAw&c=nKjWec2b6R0mOyPaz7xtfQ&r=Zta64bwn4nurTRpD4LY2OGr8KklkMRPn7Z_Qy0o4unU&m=ZMaAF9tkNCKfk8gYXMiPERZeIhavaQ7MdKtqCbShRF6w5ISrgHqAl6XDOONYbprZ&s=ERVX40JlAgnvRvPLm8sYZalsYDbpUU7YAqI0Ol0faPI&e= > > for example > > ss - Apologies. Will be careful. > > Jonathan > > On Thu, 8 Jun 2023 11:31:53 +0100 > Jonathan Cameron wrote: > > > On Wed, 7 Jun 2023 23:01:11 + > > Shesha Bhushan Sreenivasamurthy wrote: > > > > > Hi, > > > For DCD sideband there needs to be LD-ID. Is the following approach > > > acceptable? > > > > QEMU question so +CC qemu-devel > > > > > > > > -device > > > cxl-type3,bus=swport0,volatile-memdev=vmem0,dc-memdev=vmem1,id=cxl-vmem0,num-dc-regions=2,ldid=0 > > > \ > > > -device > > > cxl-type3,bus=swport0,volatile-memdev=vmem1,dc-memdev=vmem2,id=cxl-vmem1,num-dc-regions=2,ldid=1 > > > \ > > > > Those will be PCI functions at this level so you can't do this until we > > have more advanced switch support > > (it has to know about multiple VHs - right now we only support fixed config > > switches). You could connect them > > to different switch ports - effectively that will be what it looks like > > when we do emulate a configurable switch. > > > > > -device > > > i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=24,target=cxl-vmem0,cxl-vmem1") > > > > > > With this configuration, the same i2c device is handing both LDs and in > > > FMAPI commands we use the LDID specified above. > > > > This effectively becomes a partial implementation of either an MLD or an > > MH-SLD > > To manage the actual memory access, those will almost certainly need a > > bunch of other shared > > infrastructure. So I'd ultimately expect the i2c_mctp_cxl device to target > > whatever > > device represents that shared infrastructure - it might be a separate > > device or a 'lead' type 3 device. > > > > So I'm not sure how this will fit together longer term. We need same > > infrastructure > > to work for a mailbox CCI on a MH-SLD/MLD as well and in that case there > > isn't a separate > > device to which we can provide multiple targets as you've done in your > > proposal here. > > > > So I think we need to work out how to handle all of (I've probably > > forgotten something) > > X marks done or in progress. > > > > X 1) i2c_mctp_cxl to an SLD (no PCI Mailbox definition for this one) > > 2) i2c_mctp_cxl directly to an MLD (your case) > > X 3) i2c_mctp_cxl to a fixed config switch (single fixed VH no MLD capable > > ports) > > X 4) PCI mailbox via switch CCI device that fixed config switch (no MLD > > capable ports) > > Even with this simple design some fun things you can do. > > 5) i2c_mctp_cxl to a configurable switch (probably a separate as yet to > >be defined management interface - that messes with hotplug) > > 6) PCI mailbox via switch CCI to configurable switch (again to defined > >management interface). > > 7) i2c_mctp_cxl to an MH-SLD - probably to which ever device also has > >support for > > tunneling to
[PATCH v3 2/8] hw/ide/core: set ERR_STAT in unsupported command completion
From: Niklas Cassel
Currently, the first time sending an unsupported command
(e.g. READ LOG DMA EXT) will not have ERR_STAT set in the completion.
Sending the unsupported command again, will correctly have ERR_STAT set.
When ide_cmd_permitted() returns false, it calls ide_abort_command().
ide_abort_command() first calls ide_transfer_stop(), which will call
ide_transfer_halt() and ide_cmd_done(), after that ide_abort_command()
sets ERR_STAT in status.
ide_cmd_done() for AHCI will call ahci_write_fis_d2h() which writes the
current status in the FIS, and raises an IRQ. (The status here will not
have ERR_STAT set!).
Thus, we cannot call ide_transfer_stop() before setting ERR_STAT, as
ide_transfer_stop() will result in the FIS being written and an IRQ
being raised.
The reason why it works the second time, is that ERR_STAT will still
be set from the previous command, so when writing the FIS, the
completion will correctly have ERR_STAT set.
Set ERR_STAT before writing the FIS (calling cmd_done), so that we will
raise an error IRQ correctly when receiving an unsupported command.
Signed-off-by: Niklas Cassel
---
hw/ide/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index de48ff9f86..07971c0218 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -533,9 +533,9 @@ BlockAIOCB *ide_issue_trim(
void ide_abort_command(IDEState *s)
{
-ide_transfer_stop(s);
s->status = READY_STAT | ERR_STAT;
s->error = ABRT_ERR;
+ide_transfer_stop(s);
}
static void ide_set_retry(IDEState *s)
--
2.40.1
Re: Reducing vdpa migration downtime because of memory pin / maps
On Fri, Jun 9, 2023 at 12:39 AM Si-Wei Liu wrote: > > > On 6/7/23 01:08, Eugenio Perez Martin wrote: > > On Wed, Jun 7, 2023 at 12:43 AM Si-Wei Liu wrote: > >> Sorry for reviving this old thread, I lost the best timing to follow up > >> on this while I was on vacation. I have been working on this and found > >> out some discrepancy, please see below. > >> > >> On 4/5/23 04:37, Eugenio Perez Martin wrote: > >>> Hi! > >>> > >>> As mentioned in the last upstream virtio-networking meeting, one of > >>> the factors that adds more downtime to migration is the handling of > >>> the guest memory (pin, map, etc). At this moment this handling is > >>> bound to the virtio life cycle (DRIVER_OK, RESET). In that sense, the > >>> destination device waits until all the guest memory / state is > >>> migrated to start pinning all the memory. > >>> > >>> The proposal is to bind it to the char device life cycle (open vs > >>> close), > >> Hmmm, really? If it's the life cycle for char device, the next guest / > >> qemu launch on the same vhost-vdpa device node won't make it work. > >> > > Maybe my sentence was not accurate, but I think we're on the same page here. > > > > Two qemu instances opening the same char device at the same time are > > not allowed, and vhost_vdpa_release clean all the maps. So the next > > qemu that opens the char device should see a clean device anyway. > > I mean the pin can't be done at the time of char device open, where the > user address space is not known/bound yet. The earliest point possible > for pinning would be until the vhost_attach_mm() call from SET_OWNER is > done. Maybe we are deviating, let me start again. Using QEMU code, what I'm proposing is to modify the lifecycle of the .listener member of struct vhost_vdpa. At this moment, the memory listener is registered at vhost_vdpa_dev_start(dev, started=true) call for the last vhost_dev, and is unregistered in both vhost_vdpa_reset_status and vhost_vdpa_cleanup. My original proposal was just to move the memory listener registration to the last vhost_vdpa_init, and remove the unregister from vhost_vdpa_reset_status. The calls to vhost_vdpa_dma_map/unmap would be the same, the device should not realize this change. One of the concerns was that it could delay VM initialization, and I didn't profile it but I think that may be the case. I'm not sure about the right fix but I think the change is easy to profile. If that is the case, we could: * use a flag (listener->address_space ?) and only register the listener in _init if waiting for a migration, do it in _start otherwise. * something like io_uring, where the map can be done in parallel and we can fail _start if some of them fails. > Actually I think the counterpart vhost_detach_mm() only gets > handled in vhost_vdpa_release() at device close time is a resulting > artifact and amiss of today's vhost protocol - the opposite RESET_OWNER > call is not made mandatory hence only seen implemented in vhost-net > device today. One qemu instance could well exec(3) another new qemu > instance to live upgrade itself while keeping all emulated devices and > guest alive. The current vhost design simply prohibits this from happening. > Ok, I was not aware of this. Thanks for explaining it! > > > > >>>so all the guest memory can be pinned for all the guest / qemu > >>> lifecycle. > >> I think to tie pinning to guest / qemu process life cycle makes more > >> sense. Essentially this pinning part needs to be decoupled from the > >> iotlb mapping abstraction layer, and can / should work as a standalone > >> uAPI. Such that QEMU at the destination may launch and pin all guest's > >> memory as needed without having to start the device, while awaiting any > >> incoming migration request. Though problem is, there's no existing vhost > >> uAPI that could properly serve as the vehicle for that. SET_OWNER / > >> SET_MEM_TABLE / RESET_OWNER seems a remote fit.. Any objection against > >> introducing a new but clean vhost uAPI for pinning guest pages, subject > >> to guest's life cycle? > >> > > I think that to pin or not pin memory maps should be a kernel > > decision, not to be driven by qemu. > > It's kernel decision for sure. I am with this part. > > > I'm not against it if needed, but > > let me know if the current "clean at close" address your concerns. > > To better facilitate QEMU exec (live update) case, I propose we add new > vhost uAPI pair for explicit pinning request - which would live with > user mm's, or more precisely qemu instance's lifecycle. > Ok I see your problem better now, but I think it should be solved at kernel level. Does that live update need to forcefully unpin and pin the memory again, or that is just a consequence of how it works the memory listener right now? Why not extend the RESET_OWNER to the rest of devices? It seems the most natural way to me. Thanks! > > > >> Another concern is the use_va stuff, originally it tags to the device > >> level and is made static at t
[PATCH v3 4/8] hw/ide/ahci: simplify and document PxCI handling
From: Niklas Cassel
The AHCI spec states that:
For NCQ, PxCI is cleared on command queued successfully.
For non-NCQ, PxCI is cleared on command completed successfully.
(A non-NCQ command that completes with error does not clear PxCI.)
The current QEMU implementation either clears PxCI in check_cmd(),
or in ahci_cmd_done().
check_cmd() will clear PxCI for a command if handle_cmd() returns 0.
handle_cmd() will return -1 if BUSY or DRQ is set.
The QEMU implementation for NCQ commands will currently not set BUSY
or DRQ, so they will always have PxCI cleared by handle_cmd().
ahci_cmd_done() will never even get called for NCQ commands.
Non-NCQ commands are executed by ide_bus_exec_cmd().
Non-NCQ commands in QEMU are implemented either in a sync or in an async
way.
For non-NCQ commands implemented in a sync way, the command handler will
return true, and when ide_bus_exec_cmd() sees that a command handler
returns true, it will call ide_cmd_done() (which will call
ahci_cmd_done()). For a command implemented in a sync way,
ahci_cmd_done() will do nothing (since busy_slot is not set). Instead,
after ide_bus_exec_cmd() has finished, check_cmd() will clear PxCI for
these commands.
For non-NCQ commands implemented in an async way (using either aiocb or
pio_aiocb), the command handler will return false, ide_bus_exec_cmd()
will not call ide_cmd_done(), instead it is expected that the async
callback function will call ide_cmd_done() once the async command is
done. handle_cmd() will set busy_slot, if and only if BUSY or DRQ is
set, and this is checked _after_ ide_bus_exec_cmd() has returned.
handle_cmd() will return -1, so check_cmd() will not clear PxCI.
When the async callback calls ide_cmd_done() (which will call
ahci_cmd_done()), it will see that busy_slot is set, and
ahci_cmd_done() will clear PxCI.
This seems racy, since busy_slot is set _after_ ide_bus_exec_cmd() has
returned. The callback might come before busy_slot gets set. And it is
quite confusing that ahci_cmd_done() will be called for all non-NCQ
commands when the command is done, but will only clear PxCI in certain
cases, even though it will always write a D2H FIS and raise an IRQ.
Even worse, in the case where ahci_cmd_done() does not clear PxCI, it
still raises an IRQ. Host software might thus read an old PxCI value,
since PxCI is cleared (by check_cmd()) after the IRQ has been raised.
Try to simplify this by always setting busy_slot for non-NCQ commands,
such that ahci_cmd_done() will always be responsible for clearing PxCI
for non-NCQ commands.
For NCQ commands, clear PxCI when we receive the D2H FIS, but before
raising the IRQ, see AHCI 1.3.1, section 5.3.8, states RegFIS:Entry and
RegFIS:ClearCI.
Signed-off-by: Niklas Cassel
---
hw/ide/ahci.c | 70 ---
1 file changed, 50 insertions(+), 20 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 4b272397fd..3deaf01add 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -41,9 +41,10 @@
#include "trace.h"
static void check_cmd(AHCIState *s, int port);
-static int handle_cmd(AHCIState *s, int port, uint8_t slot);
+static void handle_cmd(AHCIState *s, int port, uint8_t slot);
static void ahci_reset_port(AHCIState *s, int port);
static bool ahci_write_fis_d2h(AHCIDevice *ad, bool d2h_fis_i);
+static void ahci_clear_cmd_issue(AHCIDevice *ad, uint8_t slot);
static void ahci_init_d2h(AHCIDevice *ad);
static int ahci_dma_prepare_buf(const IDEDMA *dma, int32_t limit);
static bool ahci_map_clb_address(AHCIDevice *ad);
@@ -591,9 +592,8 @@ static void check_cmd(AHCIState *s, int port)
if ((pr->cmd & PORT_CMD_START) && pr->cmd_issue) {
for (slot = 0; (slot < 32) && pr->cmd_issue; slot++) {
-if ((pr->cmd_issue & (1U << slot)) &&
-!handle_cmd(s, port, slot)) {
-pr->cmd_issue &= ~(1U << slot);
+if (pr->cmd_issue & (1U << slot)) {
+handle_cmd(s, port, slot);
}
}
}
@@ -1123,6 +1123,22 @@ static void process_ncq_command(AHCIState *s, int port,
const uint8_t *cmd_fis,
return;
}
+/*
+ * A NCQ command clears the bit in PxCI after the command has been QUEUED
+ * successfully (ERROR not set, BUSY and DRQ cleared).
+ *
+ * For NCQ commands, PxCI will always be cleared here.
+ *
+ * (Once the NCQ command is COMPLETED, the device will send a SDB FIS with
+ * the interrupt bit set, which will clear PxSACT and raise an interrupt.)
+ */
+ahci_clear_cmd_issue(ad, slot);
+
+/*
+ * In reality, for NCQ commands, PxCI is cleared after receiving a D2H FIS
+ * without the interrupt bit set, but since ahci_write_fis_d2h() can raise
+ * an IRQ on error, we need to call them in reverse order.
+ */
ahci_write_fis_d2h(ad, false);
ncq_tfs->used = 1;
@@ -1197,6 +1213,7 @@ static void handle_reg_h2d_fis(AHCIState *s, int port,
{
IDEState *ide_state = &s->dev[port].por
[PATCH v3 7/8] hw/ide/ahci: fix ahci_write_fis_sdb()
From: Niklas Cassel
When there is an error, we need to raise a TFES error irq, see AHCI 1.3.1,
5.3.13.1 SDB:Entry.
If ERR_STAT is set, we jump to state ERR:FatalTaskfile, which will raise
a TFES IRQ unconditionally, regardless if the I bit is set in the FIS or
not.
Thus, we should never raise a normal IRQ after having sent an error IRQ.
It is valid to signal successfully completed commands as finished in the
same SDB FIS that generates the error IRQ. The important thing is that
commands that did not complete successfully (e.g. commands that were
aborted, do not get the finished bit set).
Before this commit, there was never a TFES IRQ raised on NCQ error.
Signed-off-by: Niklas Cassel
---
hw/ide/ahci.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 12aaadc554..ef6c9fc378 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -806,8 +806,14 @@ static void ahci_write_fis_sdb(AHCIState *s,
NCQTransferState *ncq_tfs)
pr->scr_act &= ~ad->finished;
ad->finished = 0;
-/* Trigger IRQ if interrupt bit is set (which currently, it always is) */
-if (sdb_fis->flags & 0x40) {
+/*
+ * TFES IRQ is always raised if ERR_STAT is set, regardless of I bit.
+ * If ERR_STAT is not set, trigger SDBS IRQ if interrupt bit is set
+ * (which currently, it always is).
+ */
+if (sdb_fis->status & ERR_STAT) {
+ahci_trigger_irq(s, ad, AHCI_PORT_IRQ_BIT_TFES);
+} else if (sdb_fis->flags & 0x40) {
ahci_trigger_irq(s, ad, AHCI_PORT_IRQ_BIT_SDBS);
}
}
--
2.40.1
Re: [PATCH v3 4/5] intel_iommu: Fix address space unmap
On Fri, Jun 09, 2023 at 03:31:46AM +, Duan, Zhenzhong wrote: > > > >-Original Message- > >From: Peter Xu > >Sent: Thursday, June 8, 2023 9:48 PM > >To: Duan, Zhenzhong > >Cc: qemu-devel@nongnu.org; m...@redhat.com; jasow...@redhat.com; > >pbonz...@redhat.com; richard.hender...@linaro.org; edua...@habkost.net; > >marcel.apfelb...@gmail.com; alex.william...@redhat.com; > >c...@redhat.com; da...@redhat.com; phi...@linaro.org; > >kwankh...@nvidia.com; c...@nvidia.com; Liu, Yi L ; Peng, > >Chao P > >Subject: Re: [PATCH v3 4/5] intel_iommu: Fix address space unmap > > > >On Thu, Jun 08, 2023 at 05:52:30PM +0800, Zhenzhong Duan wrote: > >> During address space unmap, corresponding IOVA tree entries are also > >> removed. But DMAMap is set beyond notifier's scope by 1, so in theory > >> there is possibility to remove a continuous entry above the notifier's > >> scope but falling in adjacent notifier's scope. > > > >This function is only called in "loop over all notifiers" case (or replay() > >that just > >got removed, but even so there'll be only 1 notifier normally iiuc at least > >for > >vt-d), hopefully it means no bug exist (no Fixes needed, no backport needed > >either), but still worth fixing it up. > > Not two notifiers as vtd-ir splits for vt-d? The two notifiers will all be attached to the same IOMMU mr, so IOMMU_NOTIFIER_FOREACH() will loop over them all always? And this actually shouldn't matter, IMHO, as the IR split has the 0xfeeX hole only, so when notifying with end=0xfee0 (comparing to end=0xfedf) it shouldn't make a difference iiuc because there should have no iova entry at 0xfee0 anyway in the tree. -- Peter Xu
Re: [PATCH] build: further refine build.ninja rules
On 8/6/23 15:43, Paolo Bonzini wrote:
In commit b0fcc6fc7fc1 ("build: rebuild build.ninja using
"meson setup --reconfigure"", 2023-05-19) I changed the build.ninja
rule in the Makefile to use "meson setup" so that the Makefile would
pick up a changed path to the meson binary.
However, there was a reason why build.ninja was rebuilt using $(NINJA)
itself. Namely, ninja has its own cache of file modification times,
and if it does not know about the modification that was done outside
its control, it will *also* try to regenerate build.ninja. This can be
simply by running "make" on a fresh tree immediately after "configure";
that will trigger an unnecessary meson run.
So, apply a refinement to the rule in order to cover both cases:
- track the meson binary that was used (and that is embedded in
build.ninja's reconfigure rules); to do this, write build.ninja.stamp
right after executing meson successfully
- if it changed, force usage of "$(MESON) setup --reconfigure" to
update the path in the reconfigure rule
- if it didn't change, use "$(NINJA) build.ninja" just like before
commit b0fcc6fc7fc1.
Reported-by: Mark Cave-Ayland
Signed-off-by: Paolo Bonzini
---
Makefile | 17 +
configure | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
Tested-by: Philippe Mathieu-Daudé
[PATCH v2 2/2] qmp: update virtio feature maps, vhost-user-gpio instrospection
Add new virtio transport feature to transport feature map:
- VIRTIO_F_RING_RESET
Add new vhost-user protocol feature to vhost-user protocol feature map
and enumeration:
- VHOST_USER_PROTOCOL_F_STATUS
Add new virtio device features for several virtio devices to their
respective feature mappings:
virtio-blk:
- VIRTIO_BLK_F_SECURE_ERASE
virtio-net:
- VIRTIO_NET_F_NOTF_COAL
- VIRTIO_NET_F_GUEST_USO4
- VIRTIO_NET_F_GUEST_USO6
- VIRTIO_NET_F_HOST_USO
virtio/vhost-user-gpio:
- VIRTIO_GPIO_F_IRQ
- VHOST_F_LOG_ALL
- VHOST_USER_F_PROTOCOL_FEATURES
virtio-bt:
- VIRTIO_BT_F_VND_HCI
- VIRTIO_BT_F_MSFT_EXT
- VIRTIO_BT_F_AOSP_EXT
- VIRTIO_BT_F_CONFIG_V2
virtio-scmi:
- VIRTIO_SCMI_F_P2A_CHANNELS
- VIRTIO_SCMI_F_SHARED_MEMORY
Add support for introspection on vhost-user-gpio devices.
Signed-off-by: Jonah Palmer
---
hw/virtio/vhost-user-gpio.c | 7
hw/virtio/virtio-qmp.c | 79 +++--
2 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index d6927b610a..e88ca5370f 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -205,6 +205,12 @@ static void vu_gpio_guest_notifier_mask(VirtIODevice
*vdev, int idx, bool mask)
vhost_virtqueue_mask(&gpio->vhost_dev, vdev, idx, mask);
}
+static struct vhost_dev *vu_gpio_get_vhost(VirtIODevice *vdev)
+{
+VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
+return &gpio->vhost_dev;
+}
+
static void do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserGPIO *gpio)
{
virtio_delete_queue(gpio->command_vq);
@@ -413,6 +419,7 @@ static void vu_gpio_class_init(ObjectClass *klass, void
*data)
vdc->get_config = vu_gpio_get_config;
vdc->set_status = vu_gpio_set_status;
vdc->guest_notifier_mask = vu_gpio_guest_notifier_mask;
+vdc->get_vhost = vu_gpio_get_vhost;
}
static const TypeInfo vu_gpio_info = {
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index e936cc8ce5..140c420d87 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -53,6 +53,7 @@ enum VhostUserProtocolFeature {
VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
+VHOST_USER_PROTOCOL_F_STATUS = 16,
VHOST_USER_PROTOCOL_F_MAX
};
@@ -79,6 +80,8 @@ static const qmp_virtio_feature_map_t virtio_transport_map[]
= {
"VIRTIO_F_ORDER_PLATFORM: Memory accesses ordered by platform"),
FEATURE_ENTRY(VIRTIO_F_SR_IOV, \
"VIRTIO_F_SR_IOV: Device supports single root I/O virtualization"),
+FEATURE_ENTRY(VIRTIO_F_RING_RESET, \
+"VIRTIO_F_RING_RESET: Driver can reset individual VQs"),
/* Virtio ring transport features */
FEATURE_ENTRY(VIRTIO_RING_F_INDIRECT_DESC, \
"VIRTIO_RING_F_INDIRECT_DESC: Indirect descriptors supported"),
@@ -134,6 +137,9 @@ static const qmp_virtio_feature_map_t
vhost_user_protocol_map[] = {
FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS, \
"VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS: Configuration for "
"memory slots supported"),
+FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_STATUS, \
+"VHOST_USER_PROTOCOL_F_STATUS: Querying and notifying back-end "
+"device statuses supported"),
{ -1, "" }
};
@@ -176,6 +182,8 @@ static const qmp_virtio_feature_map_t
virtio_blk_feature_map[] = {
"VIRTIO_BLK_F_DISCARD: Discard command supported"),
FEATURE_ENTRY(VIRTIO_BLK_F_WRITE_ZEROES, \
"VIRTIO_BLK_F_WRITE_ZEROES: Write zeroes command supported"),
+FEATURE_ENTRY(VIRTIO_BLK_F_SECURE_ERASE, \
+"VIRTIO_BLK_F_SECURE_ERASE: Secure erase supported"),
FEATURE_ENTRY(VIRTIO_BLK_F_ZONED, \
"VIRTIO_BLK_F_ZONED: Zoned block devices"),
#ifndef VIRTIO_BLK_NO_LEGACY
@@ -299,6 +307,14 @@ static const qmp_virtio_feature_map_t
virtio_net_feature_map[] = {
FEATURE_ENTRY(VIRTIO_NET_F_CTRL_MAC_ADDR, \
"VIRTIO_NET_F_CTRL_MAC_ADDR: MAC address set through control "
"channel"),
+FEATURE_ENTRY(VIRTIO_NET_F_NOTF_COAL, \
+"VIRTIO_NET_F_NOTF_COAL: Device supports coalescing
notifications"),
+FEATURE_ENTRY(VIRTIO_NET_F_GUEST_USO4, \
+"VIRTIO_NET_F_GUEST_USO4: Driver can receive USOv4"),
+FEATURE_ENTRY(VIRTIO_NET_F_GUEST_USO6, \
+"VIRTIO_NET_F_GUEST_USO4: Driver can receive USOv6"),
+FEATURE_ENTRY(VIRTIO_NET_F_HOST_USO, \
+"VIRTIO_NET_F_HOST_USO: Device can receive USO"),
FEATURE_ENTRY(VIRTIO_NET_F_HASH_REPORT, \
"VIRTIO_NET_F_HASH_REPORT: Hash reporting supported"),
FEATURE_ENTRY(VIRTIO_NET_F_RSS, \
@@ -469,6 +485,48 @@ static const qmp_virtio_feature_map_t
virtio_rng_feature_map[] = {
};
#endif
+/* virtio/vhost-gpio features mapping */
+#ifdef CONFIG_VIRTIO_GPIO
+static const qmp_virtio_feature_map_t virtio_gpio_feature_
Re: [PATCH 00/22] target/arm/tcg: Remove inclusions of 'exec/helper-[proto/gen].h'
On 9/6/23 12:46, Philippe Mathieu-Daudé wrote: Hi, This series tries to do what Richard suggested in [*]: Targets that have a tcg/ subdir should have helper.h.inc moved into there. At the end, include/exec/helper-gen.h and include/exec/helper-proto.h become unused and go away. but only for the ARM target, to first see if this is heading in the correct direction. Instead of having TCG generated helpers included by all translate/helper units, they are reduced to the few places really needing them. I expect the build step to be slighly improved, since the compiler doesn't have to compile and track unused helpers definitions. 42 files changed, 2100 insertions(+), 1804 deletions(-) create mode 100644 target/arm/tcg/helper-iwmmxt.h.inc create mode 100644 target/arm/tcg/helper-neon.h.inc create mode 100644 target/arm/tcg/helper-vfp.h.inc create mode 100644 target/arm/tcg/translate-iwmmxt.c Note to myself, these new files miss a license. Perhaps we could have a check for SPDX in checkpatch.pl, along with "added, moved or deleted file(s), does MAINTAINERS need updating?".
