Re: [PATCH v3 3/8] ppc/pnv: PNV I2C engines assigned incorrect XSCOM addresses
Nick,
Since I started collecting fixes for 8.2 while you were away, I will
finish this cycle with a last PR next week and let you take over 9.0.
On 11/14/23 20:56, Glenn Miles wrote:
The PNV I2C engines for power9 and power10 were being assigned a base
XSCOM address that was off by one I2C engine's address range such
that engine 0 had engine 1's address and so on. The xscom address
assignment was being based on the device tree engine numbering, which
starts at 1. Rather than changing the device tree numbering to start
with 0, the addressing was changed to be based on the existing device
tree numbers minus one.
Fixes: 1ceda19c28a1 ("ppc/pnv: Connect PNV I2C controller to powernv10)
Signed-off-by: Glenn Miles
---
Changes from v2:
- Added Fixes: tag
Applied to ppc-next.
Thanks,
C.
hw/ppc/pnv.c | 6 --
hw/ppc/pnv_i2c.c | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 645379d5bf..e82e9b30ec 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1623,7 +1623,8 @@ static void pnv_chip_power9_realize(DeviceState *dev,
Error **errp)
return;
}
pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
- chip9->i2c[i].engine * PNV9_XSCOM_I2CM_SIZE,
+(chip9->i2c[i].engine - 1) *
+PNV9_XSCOM_I2CM_SIZE,
>i2c[i].xscom_regs);
qdev_connect_gpio_out(DEVICE(>i2c[i]), 0,
qdev_get_gpio_in(DEVICE(>psi),
@@ -1871,7 +1872,8 @@ static void pnv_chip_power10_realize(DeviceState *dev,
Error **errp)
return;
}
pnv_xscom_add_subregion(chip, PNV10_XSCOM_I2CM_BASE +
-chip10->i2c[i].engine * PNV10_XSCOM_I2CM_SIZE,
+(chip10->i2c[i].engine - 1) *
+PNV10_XSCOM_I2CM_SIZE,
>i2c[i].xscom_regs);
qdev_connect_gpio_out(DEVICE(>i2c[i]), 0,
qdev_get_gpio_in(DEVICE(>psi),
diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index f75e59e709..b2c738da50 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -593,7 +593,7 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void
*fdt,
int i2c_offset;
const char i2c_compat[] = "ibm,power8-i2cm\0ibm,power9-i2cm";
uint32_t i2c_pcba = PNV9_XSCOM_I2CM_BASE +
-i2c->engine * PNV9_XSCOM_I2CM_SIZE;
+(i2c->engine - 1) * PNV9_XSCOM_I2CM_SIZE;
uint32_t reg[2] = {
cpu_to_be32(i2c_pcba),
cpu_to_be32(PNV9_XSCOM_I2CM_SIZE)
Re: [PATCH v3 4/8] ppc/pnv: Fix PNV I2C invalid status after reset
On 11/14/23 20:56, Glenn Miles wrote:
The PNV I2C Controller was clearing the status register
after a reset without repopulating the "upper threshold
for I2C ports", "Command Complete" and the SCL/SDA input
level fields.
Fixed this for resets caused by a system reset as well
as from writing to the "Immediate Reset" register.
Fixes: 263b81ee15af ("ppc/pnv: Add an I2C controller model")
Signed-off-by: Glenn Miles
---
Applied to ppc-next.
Thanks,
C.
Changes from v2:
-Added Fixes: tag
hw/ppc/pnv_i2c.c | 42 ++
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index b2c738da50..f80589157b 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -462,6 +462,23 @@ static uint64_t pnv_i2c_xscom_read(void *opaque, hwaddr
addr,
return val;
}
+static void pnv_i2c_reset(void *dev)
+{
+PnvI2C *i2c = PNV_I2C(dev);
+
+memset(i2c->regs, 0, sizeof(i2c->regs));
+
+i2c->regs[I2C_STAT_REG] =
+SETFIELD(I2C_STAT_UPPER_THRS, 0ull, i2c->num_busses - 1) |
+I2C_STAT_CMD_COMP | I2C_STAT_SCL_INPUT_LEVEL |
+I2C_STAT_SDA_INPUT_LEVEL;
+i2c->regs[I2C_EXTD_STAT_REG] =
+SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
+SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
+
+fifo8_reset(>fifo);
+}
+
static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
uint64_t val, unsigned size)
{
@@ -499,16 +516,7 @@ static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
break;
case I2C_RESET_I2C_REG:
-i2c->regs[I2C_MODE_REG] = 0;
-i2c->regs[I2C_CMD_REG] = 0;
-i2c->regs[I2C_WATERMARK_REG] = 0;
-i2c->regs[I2C_INTR_MASK_REG] = 0;
-i2c->regs[I2C_INTR_COND_REG] = 0;
-i2c->regs[I2C_INTR_RAW_COND_REG] = 0;
-i2c->regs[I2C_STAT_REG] = 0;
-i2c->regs[I2C_RESIDUAL_LEN_REG] = 0;
-i2c->regs[I2C_EXTD_STAT_REG] &=
-(I2C_EXTD_STAT_FIFO_SIZE | I2C_EXTD_STAT_I2C_VERSION);
+pnv_i2c_reset(i2c);
break;
case I2C_RESET_ERRORS:
@@ -620,20 +628,6 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void
*fdt,
return 0;
}
-static void pnv_i2c_reset(void *dev)
-{
-PnvI2C *i2c = PNV_I2C(dev);
-
-memset(i2c->regs, 0, sizeof(i2c->regs));
-
-i2c->regs[I2C_STAT_REG] = I2C_STAT_CMD_COMP;
-i2c->regs[I2C_EXTD_STAT_REG] =
-SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
-SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
-
-fifo8_reset(>fifo);
-}
-
static void pnv_i2c_realize(DeviceState *dev, Error **errp)
{
PnvI2C *i2c = PNV_I2C(dev);
Re: [PATCH v3] hw/ide/ahci: fix legacy software reset
10.11.2023 12:51, Kevin Wolf:
Am 08.11.2023 um 23:26 hat Niklas Cassel geschrieben:
From: Niklas Cassel
Legacy software contains a standard mechanism for generating a reset to a
Serial ATA device - setting the SRST (software reset) bit in the Device
Control register.
...
Fixes: e2a5d9b3d9c3 ("hw/ide/ahci: simplify and document PxCI handling")
Reported-by: Marcin Juszkiewicz
Signed-off-by: Niklas Cassel
Thanks, applied to the block branch.
Another friendly ping?
This change, once again, missed the next stable-8.1.x release, it seems
(the deadline for which is tomorrow).
/mjt
Re: [PATCH v5 00/31] Unified CPU type check
On 11/17/23 17:34, Philippe Mathieu-Daudé wrote: On 17/11/23 00:26, Gavin Shan wrote: On 11/17/23 02:20, Philippe Mathieu-Daudé wrote: On 16/11/23 14:35, Philippe Mathieu-Daudé wrote: I'm queuing patches 1-3 & 5-23 to my cpus-next tree. No need to repost them, please base them on my tree. I'll follow up with the branch link when I finish my testing and push it. Here are these patches queued: https://github.com/philmd/qemu.git branches/cpus-next Oops, no clue why I wrote github instead of gitlab, sorry =) No worries, Phil. I might queue more patches before the 9.0 merge window opens. Thanks for queuing these patches, but I don't see 'cpus-next' branch in the repository. Please let me know if I checked out the code properly. $ git clone https://github.com/philmd/qemu.git philmd $ cd philmd $ git branch * staging $ git branch -a | grep cpus-next $ echo $? 1 No need to clone, you can use in your current cloned repository: $ git fetch https://gitlab.com/philmd/qemu.git cpus-next:cpus-next Thanks. It worked for me. Thanks, Gavin
[PATCH] qga/linux: Add new api 'guest-network-get-route'
The Route information of the Linux VM needs to be used
by administrators and users when debugging network problems
and troubleshooting.
Signed-off-by: Dehan Meng
---
qga/commands-posix.c | 82
qga/commands-win32.c | 6
qga/qapi-schema.json | 80 ++
3 files changed, 168 insertions(+)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 6169bbf7a0..83a3c46b3c 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2747,6 +2747,82 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
return head;
}
+char *hexToIPAddress(unsigned int hexValue, char ipAddress[16]);
+
+char *hexToIPAddress(unsigned int hexValue, char ipAddress[16])
+{
+unsigned int byte1 = (hexValue >> 24) & 0xFF;
+unsigned int byte2 = (hexValue >> 16) & 0xFF;
+unsigned int byte3 = (hexValue >> 8) & 0xFF;
+unsigned int byte4 = hexValue & 0xFF;
+
+snprintf(ipAddress, 16, "%u.%u.%u.%u", byte4, byte3, byte2, byte1);
+
+return ipAddress;
+}
+
+GuestNetworkRouteList *qmp_guest_network_get_route(Error **errp)
+{
+GuestNetworkRouteList *head = NULL, **tail =
+const char *routeFile = "/proc/net/route";
+FILE *fp;
+size_t n;
+char *line = NULL;
+
+fp = fopen(routeFile, "r");
+if (fp == NULL) {
+error_setg_errno(errp, errno, "open(\"%s\")", routeFile);
+return NULL;
+}
+
+while (getline(, , fp) != -1) {
+GuestNetworkRoute *route = NULL;
+GuestNetworkRouteStat *networkroute;
+int i;
+char Iface[16];
+unsigned int Destination, Gateway, Mask, Flags;
+int RefCnt, Use, Metric, MTU, Window, IRTT;
+
+/* Parse the line and extract the values */
+i = (sscanf(line, "%s %X %X %x %d %d %d %X %d %d %d",
+Iface, , , , ,
+, , , , , ) == 11);
+if (i == EOF) {
+continue;
+}
+
+route = g_new0(GuestNetworkRoute, 1);
+route->type = GUEST_NETWORK_ROUTE_TYPE_LINUX;
+
+/*
+ *Additional logic to convert hex values to
+ *decimal and IP address format
+ */
+char DestAddress[16];
+char GateAddress[16];
+char MaskAddress[16];
+
+networkroute = >u.q_linux;
+networkroute->iface = g_strdup(Iface);
+networkroute->destination = hexToIPAddress(Destination, DestAddress);
+networkroute->gateway = hexToIPAddress(Gateway, GateAddress);
+networkroute->mask = hexToIPAddress(Mask, MaskAddress);
+networkroute->metric = Metric;
+networkroute->flags = Flags;
+networkroute->refcnt = RefCnt;
+networkroute->use = Use;
+networkroute->mtu = MTU;
+networkroute->window = Window;
+networkroute->irtt = IRTT;
+
+QAPI_LIST_APPEND(tail, route);
+}
+
+free(line);
+fclose(fp);
+return head;
+}
+
#else /* defined(__linux__) */
void qmp_guest_suspend_disk(Error **errp)
@@ -3118,6 +3194,12 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
return NULL;
}
+GuestNetworkRouteList *qmp_guest_network_get_route(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
+
#endif /* CONFIG_FSFREEZE */
#if !defined(CONFIG_FSTRIM)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 697c65507c..e62c04800a 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2522,3 +2522,9 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
error_setg(errp, QERR_UNSUPPORTED);
return NULL;
}
+
+GuestNetworkRouteList *qmp_guest_network_get_route(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 876e2a8ea8..eed9539bb7 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1789,3 +1789,83 @@
{ 'command': 'guest-get-cpustats',
'returns': ['GuestCpuStats']
}
+
+##
+# @GuestNetworkRouteType:
+#
+# An enumeration of OS type
+#
+# Since: 8.2
+##
+{ 'enum': 'GuestNetworkRouteType',
+ 'data': [ 'linux' ] }
+
+##
+# @GuestNetworkRouteStat:
+#
+# Route information, currently, only linux supported.
+#
+# @iface: The destination network or host's egress network interface in the
routing table
+#
+# @destination: The IP address of the target network or host, The final
destination of the packet
+#
+# @gateway: The IP address of the next hop router
+#
+# @mask: Subnet Mask
+#
+# @metric: Route metric
+#
+# @flags: Route flags (not for windows)
+#
+# @irtt: Initial round-trip delay (not for windows)
+#
+# @refcnt: The route's reference count (not for windows)
+#
+# @use: Route usage count (not for windows)
+#
+# @window: TCP window size, used for flow control (not for windows)
+#
+# @mtu: Data link layer maximum packet size (not for windows)
+#
+# Since: 8.2
+
+##
+{ 'struct': 'GuestNetworkRouteStat',
+ 'data': {'iface': 'str',
+
Re: [PATCH v3 19/70] i386/tdx: Introduce is_tdx_vm() helper and cache tdx_guest object
On Wed, Nov 15, 2023 at 02:14:28AM -0500,
Xiaoyao Li wrote:
> It will need special handling for TDX VMs all around the QEMU.
> Introduce is_tdx_vm() helper to query if it's a TDX VM.
>
> Cache tdx_guest object thus no need to cast from ms->cgs every time.
>
> Signed-off-by: Xiaoyao Li
> Acked-by: Gerd Hoffmann
> ---
> changes in v3:
> - replace object_dynamic_cast with TDX_GUEST();
> ---
> target/i386/kvm/tdx.c | 15 ++-
> target/i386/kvm/tdx.h | 10 ++
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index cb0040187b27..cf8889f0a8f9 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -21,8 +21,16 @@
> #include "kvm_i386.h"
> #include "tdx.h"
>
> +static TdxGuest *tdx_guest;
> +
> static struct kvm_tdx_capabilities *tdx_caps;
>
> +/* It's valid after kvm_confidential_guest_init()->kvm_tdx_init() */
> +bool is_tdx_vm(void)
> +{
> +return !!tdx_guest;
> +}
> +
> enum tdx_ioctl_level{
> TDX_PLATFORM_IOCTL,
> TDX_VM_IOCTL,
> @@ -114,15 +122,20 @@ static int get_tdx_capabilities(Error **errp)
>
> int tdx_kvm_init(MachineState *ms, Error **errp)
> {
> +TdxGuest *tdx = TDX_GUEST(OBJECT(ms->cgs));
> int r = 0;
>
> ms->require_guest_memfd = true;
>
> if (!tdx_caps) {
> r = get_tdx_capabilities(errp);
> +if (r) {
> +return r;
> +}
> }
>
> -return r;
> +tdx_guest = tdx;
> +return 0;
> }
>
> /* tdx guest */
> diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
> index c8a23d95258d..4036ca2f3f99 100644
> --- a/target/i386/kvm/tdx.h
> +++ b/target/i386/kvm/tdx.h
> @@ -1,6 +1,10 @@
> #ifndef QEMU_I386_TDX_H
> #define QEMU_I386_TDX_H
>
> +#ifndef CONFIG_USER_ONLY
> +#include CONFIG_DEVICES /* CONFIG_TDX */
> +#endif
> +
> #include "exec/confidential-guest-support.h"
>
> #define TYPE_TDX_GUEST "tdx-guest"
> @@ -16,6 +20,12 @@ typedef struct TdxGuest {
> uint64_t attributes;/* TD attributes */
> } TdxGuest;
>
> +#ifdef CONFIG_TDX
> +bool is_tdx_vm(void);
> +#else
> +#define is_tdx_vm() 0
> +#endif /* CONFIG_TDX */
> +
> int tdx_kvm_init(MachineState *ms, Error **errp);
>
> #endif /* QEMU_I386_TDX_H */
> --
> 2.34.1
>
>
Reviewed-by: Isaku Yamahata
--
Isaku Yamahata
Re: [PATCH v3 18/70] i386/tdx: Get tdx_capabilities via KVM_TDX_CAPABILITIES
On Wed, Nov 15, 2023 at 02:14:27AM -0500,
Xiaoyao Li wrote:
> KVM provides TDX capabilities via sub command KVM_TDX_CAPABILITIES of
> IOCTL(KVM_MEMORY_ENCRYPT_OP). Get the capabilities when initializing
> TDX context. It will be used to validate user's setting later.
>
> Since there is no interface reporting how many cpuid configs contains in
> KVM_TDX_CAPABILITIES, QEMU chooses to try starting with a known number
> and abort when it exceeds KVM_MAX_CPUID_ENTRIES.
>
> Besides, introduce the interfaces to invoke TDX "ioctls" at different
> scope (KVM, VM and VCPU) in preparation.
>
> Signed-off-by: Xiaoyao Li
> ---
> Changes in v3:
> - rename __tdx_ioctl() to tdx_ioctl_internal()
> - Pass errp in get_tdx_capabilities();
>
> changes in v2:
> - Make the error message more clear;
>
> changes in v1:
> - start from nr_cpuid_configs = 6 for the loop;
> - stop the loop when nr_cpuid_configs exceeds KVM_MAX_CPUID_ENTRIES;
> ---
> target/i386/kvm/kvm.c | 2 -
> target/i386/kvm/kvm_i386.h | 2 +
> target/i386/kvm/tdx.c | 102 -
> 3 files changed, 103 insertions(+), 3 deletions(-)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 7abcdebb1452..28e60c5ea4a7 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1687,8 +1687,6 @@ static int hyperv_init_vcpu(X86CPU *cpu)
>
> static Error *invtsc_mig_blocker;
>
> -#define KVM_MAX_CPUID_ENTRIES 100
> -
> static void kvm_init_xsave(CPUX86State *env)
> {
> if (has_xsave2) {
> diff --git a/target/i386/kvm/kvm_i386.h b/target/i386/kvm/kvm_i386.h
> index 55fb25fa8e2e..c3ef46a97a7b 100644
> --- a/target/i386/kvm/kvm_i386.h
> +++ b/target/i386/kvm/kvm_i386.h
> @@ -13,6 +13,8 @@
>
> #include "sysemu/kvm.h"
>
> +#define KVM_MAX_CPUID_ENTRIES 100
> +
> #ifdef CONFIG_KVM
>
> #define kvm_pit_in_kernel() \
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 621a05beeb4e..cb0040187b27 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -12,17 +12,117 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "qom/object_interfaces.h"
> +#include "sysemu/kvm.h"
>
> #include "hw/i386/x86.h"
> +#include "kvm_i386.h"
> #include "tdx.h"
>
> +static struct kvm_tdx_capabilities *tdx_caps;
> +
> +enum tdx_ioctl_level{
> +TDX_PLATFORM_IOCTL,
> +TDX_VM_IOCTL,
> +TDX_VCPU_IOCTL,
> +};
> +
> +static int tdx_ioctl_internal(void *state, enum tdx_ioctl_level level, int
> cmd_id,
> +__u32 flags, void *data)
> +{
> +struct kvm_tdx_cmd tdx_cmd;
> +int r;
> +
> +memset(_cmd, 0x0, sizeof(tdx_cmd));
> +
> +tdx_cmd.id = cmd_id;
> +tdx_cmd.flags = flags;
> +tdx_cmd.data = (__u64)(unsigned long)data;
> +
> +switch (level) {
> +case TDX_PLATFORM_IOCTL:
> +r = kvm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_OP, _cmd);
> +break;
> +case TDX_VM_IOCTL:
> +r = kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_OP, _cmd);
> +break;
> +case TDX_VCPU_IOCTL:
> +r = kvm_vcpu_ioctl(state, KVM_MEMORY_ENCRYPT_OP, _cmd);
> +break;
> +default:
> +error_report("Invalid tdx_ioctl_level %d", level);
> +exit(1);
> +}
> +
> +return r;
> +}
> +
> +static inline int tdx_platform_ioctl(int cmd_id, __u32 flags, void *data)
> +{
> +return tdx_ioctl_internal(NULL, TDX_PLATFORM_IOCTL, cmd_id, flags, data);
> +}
> +
> +static inline int tdx_vm_ioctl(int cmd_id, __u32 flags, void *data)
> +{
> +return tdx_ioctl_internal(NULL, TDX_VM_IOCTL, cmd_id, flags, data);
> +}
> +
> +static inline int tdx_vcpu_ioctl(void *vcpu_fd, int cmd_id, __u32 flags,
> + void *data)
> +{
> +return tdx_ioctl_internal(vcpu_fd, TDX_VCPU_IOCTL, cmd_id, flags, data);
> +}
As all of ioctl variants aren't used yet, we can split out them. An independent
patch to define ioctl functions.
> +
> +static int get_tdx_capabilities(Error **errp)
> +{
> +struct kvm_tdx_capabilities *caps;
> +/* 1st generation of TDX reports 6 cpuid configs */
> +int nr_cpuid_configs = 6;
> +size_t size;
> +int r;
> +
> +do {
> +size = sizeof(struct kvm_tdx_capabilities) +
> + nr_cpuid_configs * sizeof(struct kvm_tdx_cpuid_config);
> +caps = g_malloc0(size);
> +caps->nr_cpuid_configs = nr_cpuid_configs;
> +
> +r = tdx_vm_ioctl(KVM_TDX_CAPABILITIES, 0, caps);
> +if (r == -E2BIG) {
> +g_free(caps);
> +nr_cpuid_configs *= 2;
g_realloc()? Maybe a matter of preference.
Other than this, it looks good to me.
--
Isaku Yamahata
Re: [PATCH v3 09/70] physmem: Introduce ram_block_convert_range() for page conversion
On Wed, Nov 15, 2023 at 02:14:18AM -0500,
Xiaoyao Li wrote:
> It's used for discarding opposite memory after memory conversion, for
> confidential guest.
>
> When page is converted from shared to private, the original shared
> memory can be discarded via ram_block_discard_range();
>
> When page is converted from private to shared, the original private
> memory is back'ed by guest_memfd. Introduce
> ram_block_discard_guest_memfd_range() for discarding memory in
> guest_memfd.
>
> Originally-from: Isaku Yamahata
> Codeveloped-by: Xiaoyao Li
> Signed-off-by: Xiaoyao Li
> ---
> include/exec/cpu-common.h | 2 ++
> system/physmem.c | 50 +++
> 2 files changed, 52 insertions(+)
>
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 41115d891940..de728a18eef2 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -175,6 +175,8 @@ typedef int (RAMBlockIterFunc)(RAMBlock *rb, void
> *opaque);
>
> int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);
> int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length);
> +int ram_block_convert_range(RAMBlock *rb, uint64_t start, size_t length,
> +bool shared_to_private);
>
> #endif
>
> diff --git a/system/physmem.c b/system/physmem.c
> index ddfecddefcd6..cd6008fa09ad 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -3641,6 +3641,29 @@ err:
> return ret;
> }
>
> +static int ram_block_discard_guest_memfd_range(RAMBlock *rb, uint64_t start,
> + size_t length)
> +{
> +int ret = -1;
> +
> +#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
> +ret = fallocate(rb->guest_memfd, FALLOC_FL_PUNCH_HOLE |
> FALLOC_FL_KEEP_SIZE,
> +start, length);
> +
> +if (ret) {
> +ret = -errno;
> +error_report("%s: Failed to fallocate %s:%" PRIx64 " +%zx (%d)",
> + __func__, rb->idstr, start, length, ret);
> +}
> +#else
> +ret = -ENOSYS;
> +error_report("%s: fallocate not available %s:%" PRIx64 " +%zx (%d)",
> + __func__, rb->idstr, start, length, ret);
> +#endif
> +
> +return ret;
> +}
> +
> bool ramblock_is_pmem(RAMBlock *rb)
> {
> return rb->flags & RAM_PMEM;
> @@ -3828,3 +3851,30 @@ bool ram_block_discard_is_required(void)
> return qatomic_read(_block_discard_required_cnt) ||
> qatomic_read(_block_coordinated_discard_required_cnt);
> }
> +
> +int ram_block_convert_range(RAMBlock *rb, uint64_t start, size_t length,
> +bool shared_to_private)
> +{
> +if (!rb || rb->guest_memfd < 0) {
> +return -1;
> +}
> +
> +if (!QEMU_PTR_IS_ALIGNED(start, qemu_host_page_size) ||
> +!QEMU_PTR_IS_ALIGNED(length, qemu_host_page_size)) {
> +return -1;
> +}
> +
> +if (!length) {
> +return -1;
> +}
> +
> +if (start + length > rb->max_length) {
> +return -1;
> +}
> +
> +if (shared_to_private) {
> +return ram_block_discard_range(rb, start, length);
> +} else {
> +return ram_block_discard_guest_memfd_range(rb, start, length);
> +}
> +}
Originally this function issued KVM_SET_MEMORY_ATTRIBUTES, the function name
mad sense. But now it doesn't, and it issues only punch hole. We should rename
it to represent what it actually does. discard_range?
--
Isaku Yamahata
Re: [PATCH v3 05/70] kvm: Enable KVM_SET_USER_MEMORY_REGION2 for memslot
On Wed, Nov 15, 2023 at 02:14:14AM -0500,
Xiaoyao Li wrote:
> From: Chao Peng
>
> Switch to KVM_SET_USER_MEMORY_REGION2 when supported by KVM.
>
> With KVM_SET_USER_MEMORY_REGION2, QEMU can set up memory region that
> backend'ed both by hva-based shared memory and guest memfd based private
> memory.
>
> Signed-off-by: Chao Peng
> Co-developed-by: Xiaoyao Li
> Signed-off-by: Xiaoyao Li
> ---
> accel/kvm/kvm-all.c | 56 ++--
> accel/kvm/trace-events | 2 +-
> include/sysemu/kvm_int.h | 2 ++
> 3 files changed, 51 insertions(+), 9 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 9f751d4971f8..69afeb47c9c0 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -293,35 +293,69 @@ int kvm_physical_memory_addr_from_host(KVMState *s,
> void *ram,
> static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot,
> bool new)
> {
> KVMState *s = kvm_state;
> -struct kvm_userspace_memory_region mem;
> +struct kvm_userspace_memory_region2 mem;
> +static int cap_user_memory2 = -1;
> int ret;
>
> +if (cap_user_memory2 == -1) {
> +cap_user_memory2 = kvm_check_extension(s, KVM_CAP_USER_MEMORY2);
> +}
> +
> +if (!cap_user_memory2 && slot->guest_memfd >= 0) {
> +error_report("%s, KVM doesn't support KVM_CAP_USER_MEMORY2,"
> + " which is required by guest memfd!", __func__);
> +exit(1);
> +}
> +
> mem.slot = slot->slot | (kml->as_id << 16);
> mem.guest_phys_addr = slot->start_addr;
> mem.userspace_addr = (unsigned long)slot->ram;
> mem.flags = slot->flags;
> +mem.guest_memfd = slot->guest_memfd;
> +mem.guest_memfd_offset = slot->guest_memfd_offset;
>
> if (slot->memory_size && !new && (mem.flags ^ slot->old_flags) &
> KVM_MEM_READONLY) {
> /* Set the slot size to 0 before setting the slot to the desired
> * value. This is needed based on KVM commit 75d61fbc. */
> mem.memory_size = 0;
> -ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, );
> +
> +if (cap_user_memory2) {
> +ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION2, );
> +} else {
> +ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, );
> + }
> if (ret < 0) {
> goto err;
> }
> }
> mem.memory_size = slot->memory_size;
> -ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, );
> +if (cap_user_memory2) {
> +ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION2, );
> +} else {
> +ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, );
> +}
> slot->old_flags = mem.flags;
> err:
> trace_kvm_set_user_memory(mem.slot >> 16, (uint16_t)mem.slot, mem.flags,
>mem.guest_phys_addr, mem.memory_size,
> - mem.userspace_addr, ret);
> + mem.userspace_addr, mem.guest_memfd,
> + mem.guest_memfd_offset, ret);
> if (ret < 0) {
> -error_report("%s: KVM_SET_USER_MEMORY_REGION failed, slot=%d,"
> - " start=0x%" PRIx64 ", size=0x%" PRIx64 ": %s",
> - __func__, mem.slot, slot->start_addr,
> - (uint64_t)mem.memory_size, strerror(errno));
> +if (cap_user_memory2) {
> +error_report("%s: KVM_SET_USER_MEMORY_REGION2 failed,
> slot=%d,"
> +" start=0x%" PRIx64 ", size=0x%" PRIx64 ","
> +" flags=0x%" PRIx32 ", guest_memfd=%" PRId32 ","
> +" guest_memfd_offset=0x%" PRIx64 ": %s",
> +__func__, mem.slot, slot->start_addr,
> +(uint64_t)mem.memory_size, mem.flags,
> +mem.guest_memfd, (uint64_t)mem.guest_memfd_offset,
> +strerror(errno));
> +} else {
> +error_report("%s: KVM_SET_USER_MEMORY_REGION failed,
> slot=%d,"
> +" start=0x%" PRIx64 ", size=0x%" PRIx64 ": %s",
> +__func__, mem.slot, slot->start_addr,
> +(uint64_t)mem.memory_size, strerror(errno));
> +}
> }
> return ret;
> }
> @@ -477,6 +511,9 @@ static int kvm_mem_flags(MemoryRegion *mr)
> if (readonly && kvm_readonly_mem_allowed) {
> flags |= KVM_MEM_READONLY;
> }
> +if (memory_region_has_guest_memfd(mr)) {
> +flags |= KVM_MEM_PRIVATE;
> +}
Nitpick: it was renamed to KVM_MEM_GUEST_MEMFD
As long as the value is defined to same value, it doesn't matter, though.
--
Isaku Yamahata
Re: [PATCH v3 02/70] RAMBlock: Add support of KVM private guest memfd
On Wed, Nov 15, 2023 at 02:14:11AM -0500,
Xiaoyao Li wrote:
> diff --git a/system/physmem.c b/system/physmem.c
> index fc2b0fee0188..0af2213cbd9c 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -1841,6 +1841,20 @@ static void ram_block_add(RAMBlock *new_block, Error
> **errp)
> }
> }
>
> +#ifdef CONFIG_KVM
> +if (kvm_enabled() && new_block->flags & RAM_GUEST_MEMFD &&
> +new_block->guest_memfd < 0) {
> +/* TODO: to decide if KVM_GUEST_MEMFD_ALLOW_HUGEPAGE is supported */
> +uint64_t flags = 0;
> +new_block->guest_memfd =
> kvm_create_guest_memfd(new_block->max_length,
> +flags, errp);
> +if (new_block->guest_memfd < 0) {
> +qemu_mutex_unlock_ramlist();
> +return;
> +}
> +}
> +#endif
> +
We should define kvm_create_guest_memfd() stub in accel/stub/kvm-stub.c.
We can remove this #ifdef.
--
Isaku Yamahata
[PULL 1/2] target/hppa: Fix 64-bit SHRPD instruction
From: Helge Deller
When shifting the two joined 64-bit registers right, shift the upper
64-bit register to the left and the lower 64-bit register to the right
before merging them with OR.
Signed-off-by: Helge Deller
Reviewed-by: Richard Henderson
---
target/hppa/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 4a4830c3e3..3ef39b1bd7 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3438,9 +3438,9 @@ static bool trans_shrp_sar(DisasContext *ctx,
arg_shrp_sar *a)
TCGv_i64 n = tcg_temp_new_i64();
tcg_gen_xori_i64(n, cpu_sar, 63);
-tcg_gen_shl_i64(t, src2, n);
+tcg_gen_shl_i64(t, src1, n);
tcg_gen_shli_i64(t, t, 1);
-tcg_gen_shr_i64(dest, src1, cpu_sar);
+tcg_gen_shr_i64(dest, src2, cpu_sar);
tcg_gen_or_i64(dest, dest, t);
} else {
TCGv_i64 t = tcg_temp_new_i64();
--
2.41.0
[PULL 0/2] hppa64 fixes
From: Helge Deller The following changes since commit 34a5cb6d8434303c170230644b2a7c1d5781d197: Merge tag 'pull-tcg-20231114' of https://gitlab.com/rth7680/qemu into staging (2023-11-15 08:05:25 -0500) are available in the Git repository at: https://github.com/hdeller/qemu-hppa.git tags/hppa64-fixes-pull-request for you to fetch changes up to 2f926bfd5b79e6219ae65a1e530b38f37d62b384: disas/hppa: Show hexcode of instruction along with disassembly (2023-11-17 18:36:36 +0100) HPPA64 fixes for 8.2 The SHRPD patch fixes a real translation bug which then allows to boot the 64-bit Linux kernels of the Debian-11 and Debian-12 installation CDs. The second patch adds the instruction byte sequence to the assembly log. This is not an actual bug fix, but it's important since it helps a lot when trying to fix qemu translation bugs on hppa. Helge Deller (2): target/hppa: Fix 64-bit SHRPD instruction disas/hppa: Show hexcode of instruction along with disassembly disas/hppa.c| 6 +- target/hppa/translate.c | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) -- 2.41.0
[PULL 2/2] disas/hppa: Show hexcode of instruction along with disassembly
From: Helge Deller
On hppa many instructions can be expressed by different bytecodes.
To be able to debug qemu translation bugs it's therefore necessary to see the
currently executed byte codes without the need to lookup the sequence without
the full executable.
With this patch the instruction byte code is shown beside the disassembly.
Signed-off-by: Helge Deller
Reviewed-by: Richard Henderson
---
disas/hppa.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/disas/hppa.c b/disas/hppa.c
index dcf9a47f34..cce4f4aa37 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1968,6 +1968,10 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
insn = bfd_getb32 (buffer);
+ info->fprintf_func(info->stream, " %02x %02x %02x %02x ",
+(insn >> 24) & 0xff, (insn >> 16) & 0xff,
+(insn >> 8) & 0xff, insn & 0xff);
+
for (i = 0; i < NUMOPCODES; ++i)
{
const struct pa_opcode *opcode = _opcodes[i];
@@ -2826,6 +2830,6 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
return sizeof (insn);
}
}
- (*info->fprintf_func) (info->stream, "#%8x", insn);
+ info->fprintf_func(info->stream, "");
return sizeof (insn);
}
--
2.41.0
Re: [PATCH v2 2/2] disas/hppa: Show hexcode of instruction along with disassembly
On 11/17/23 09:33, Helge Deller wrote:
* Richard Henderson :
On 11/17/23 02:53, del...@kernel.org wrote:
From: Helge Deller
On hppa many instructions can be expressed by different bytecodes.
To be able to debug qemu translation bugs it's therefore necessary to see the
currently executed byte codes without the need to lookup the sequence without
the full executable.
With this patch the instruction byte code is shown beside the disassembly.
Signed-off-by: Helge Deller
---
disas/hppa.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/disas/hppa.c b/disas/hppa.c
index dcf9a47f34..38fc05acc4 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1979,6 +1979,9 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
if (opcode->arch == pa20w)
continue;
#endif
+ (*info->fprintf_func) (info->stream, " %02x %02x %02x %02x ",
+(insn >> 24) & 0xff, (insn >> 16) & 0xff,
+(insn >> 8) & 0xff, insn & 0xff);
(*info->fprintf_func) (info->stream, "%s", opcode->name);
if (!strchr ("cfCY?-+nHNZFIuv{", opcode->args[0]))
Reviewed-by: Richard Henderson
A possible improvement is to push this outside of the search loop and then
change
}
- (*info->fprintf_func) (info->stream, "#%8x", insn);
+ info->fprintf_func(info->stream, "");
return sizeof (insn);
so the byte decode is shared with the rare case of garbage in the insn stream.
Like below?
Yes, perfect, thanks.
r~
From: Helge Deller
Subject: [PATCH] disas/hppa: Show hexcode of instruction along with
disassembly
On hppa many instructions can be expressed by different bytecodes.
To be able to debug qemu translation bugs it's therefore necessary to see the
currently executed byte codes without the need to lookup the sequence without
the full executable.
With this patch the instruction byte code is shown beside the disassembly.
Signed-off-by: Helge Deller
diff --git a/disas/hppa.c b/disas/hppa.c
index dcf9a47f34..cce4f4aa37 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1968,6 +1968,10 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
insn = bfd_getb32 (buffer);
+ info->fprintf_func(info->stream, " %02x %02x %02x %02x ",
+(insn >> 24) & 0xff, (insn >> 16) & 0xff,
+(insn >> 8) & 0xff, insn & 0xff);
+
for (i = 0; i < NUMOPCODES; ++i)
{
const struct pa_opcode *opcode = _opcodes[i];
@@ -2826,6 +2830,6 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
return sizeof (insn);
}
}
- (*info->fprintf_func) (info->stream, "#%8x", insn);
+ info->fprintf_func(info->stream, "");
return sizeof (insn);
}
[PATCH for-8.2] target/arm: Fix SME FMOPA (16-bit), BFMOPA
Perform the loop increment unconditionally, not nested
within the predication.
Cc: qemu-sta...@nongnu.org
Fixes: 3916841ac75 ("target/arm: Implement FMOPA, FMOPS (widening)")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1985
Signed-off-by: Richard Henderson
---
target/arm/tcg/sme_helper.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c
index 296826ffe6..1ee2690ceb 100644
--- a/target/arm/tcg/sme_helper.c
+++ b/target/arm/tcg/sme_helper.c
@@ -1037,10 +1037,9 @@ void HELPER(sme_fmopa_h)(void *vza, void *vzn, void
*vzm, void *vpn,
m = f16mop_adj_pair(m, pcol, 0);
*a = f16_dotadd(*a, n, m, _std, _odd);
-
-col += 4;
-pcol >>= 4;
}
+col += 4;
+pcol >>= 4;
} while (col & 15);
}
row += 4;
@@ -1073,10 +1072,9 @@ void HELPER(sme_bfmopa)(void *vza, void *vzn, void *vzm,
void *vpn,
m = f16mop_adj_pair(m, pcol, 0);
*a = bfdotadd(*a, n, m);
-
-col += 4;
-pcol >>= 4;
}
+col += 4;
+pcol >>= 4;
} while (col & 15);
}
row += 4;
--
2.34.1
Re: [PATCH v3 1/8] ppc/pnv: Add pca9552 to powernv10 for PCIe hotplug power control
On Fri, 2023-11-17 at 17:04 +0100, Cédric Le Goater wrote: > > Well, I was hoping to sweep the pca9554 model under the PowerNV > > maintainership (like pca9552 is under the BMC aspeed > > maintainership). > > I did update the PowerNV list to include it, but perhaps that was > > presumptuous of me. :-) > > Well, you are the person who has the most knowledge on both and > you have access to HW to check changes ! > > > I would be ok with being added as a reviewer under the PowerNV > > list, > > but I wonder if I shouldn't have more opensource experience before > > becoming a maintainer? TBH, I have no idea what that would entail. > > For these devices, mostly acking the changes. I don't think anyone > will ask you to send PRs. This can be handled through some other > tree, PPC or Aspeed. > Ok, that doesn't sound too bad. Sign me up! > > > As for patches 3 and 4, it sounds like I should split those changes > > out > > from the current patch series so that they can make it into QEMU > > 8.2. > > Correct? > > I don't think this is needed. They can be picked in the series and > merged in the ppc tree independently. > > Thanks, > > C. > Ok, sounds good. Thanks, Glenn
[PATCH v3] hw/usb: fix xhci port notify
>From MCF5253 Reference manual
>https://www.nxp.com/docs/en/reference-manual/MCF5253RM.pdf
Host mode: Port Change Detect. The controller sets this bit to a one when on
any port a Connect Status occurs, a PortEnable/Disable Change occurs, an Over
Current Change occurs, or the Force Port Resume bit is set as theresult of a
J-K transition on the suspended port.
Signed-off-by: Nikita Ostrenkov
---
hw/usb/hcd-xhci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4b60114207..1b2f4ac721 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2627,6 +2627,7 @@ static void xhci_port_notify(XHCIPort *port, uint32_t
bits)
if (!xhci_running(port->xhci)) {
return;
}
+port->xhci->usbsts |= USBSTS_PCD;
xhci_event(port->xhci, , 0);
}
--
2.34.1
[PATCH v2] hw/usb: fix xhci port notify
>From MCF5253 Reference manual
>https://www.nxp.com/docs/en/reference-manual/MCF5253RM.pdf
Host mode: The controller sets this bit to a one when on any port a Connect
Status occurs, a PortEnable/Disable Change occurs, an Over Current Change
occurs, or the Force Port Resume bit is set as theresult of a J-K transition on
the suspended port.
Signed-off-by: Nikita Ostrenkov
---
hw/usb/hcd-xhci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4b60114207..1b2f4ac721 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2627,6 +2627,7 @@ static void xhci_port_notify(XHCIPort *port, uint32_t
bits)
if (!xhci_running(port->xhci)) {
return;
}
+port->xhci->usbsts |= USBSTS_PCD;
xhci_event(port->xhci, , 0);
}
--
2.34.1
Re: [PATCH-for-8.2] target/nios2: Deprecate the Nios II architecture
Philippe Mathieu-Daudé writes:
> See commit 9ba1caf510 ("MAINTAINERS: Mark the Nios II CPU as orphan"),
> last contribution from Chris was in 2012 [1] and Marek in 2018 [2].
>
> [1]
> https://lore.kernel.org/qemu-devel/1352607539-10455-2-git-send-email-crwu...@gmail.com/
> [2]
> https://lore.kernel.org/qemu-devel/805fc7b5-03f0-56d4-abfd-ed010d4fa...@denx.de/
>
> Signed-off-by: Philippe Mathieu-Daudé
Queued to for-8.2/random-fixes, thanks.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 2/2] disas/hppa: Show hexcode of instruction along with disassembly
* Richard Henderson :
> On 11/17/23 02:53, del...@kernel.org wrote:
> > From: Helge Deller
> >
> > On hppa many instructions can be expressed by different bytecodes.
> > To be able to debug qemu translation bugs it's therefore necessary to see
> > the
> > currently executed byte codes without the need to lookup the sequence
> > without
> > the full executable.
> > With this patch the instruction byte code is shown beside the disassembly.
> >
> > Signed-off-by: Helge Deller
> > ---
> > disas/hppa.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/disas/hppa.c b/disas/hppa.c
> > index dcf9a47f34..38fc05acc4 100644
> > --- a/disas/hppa.c
> > +++ b/disas/hppa.c
> > @@ -1979,6 +1979,9 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info
> > *info)
> > if (opcode->arch == pa20w)
> > continue;
> > #endif
> > + (*info->fprintf_func) (info->stream, " %02x %02x %02x %02x ",
> > +(insn >> 24) & 0xff, (insn >> 16) & 0xff,
> > +(insn >> 8) & 0xff, insn & 0xff);
> > (*info->fprintf_func) (info->stream, "%s", opcode->name);
> > if (!strchr ("cfCY?-+nHNZFIuv{", opcode->args[0]))
>
> Reviewed-by: Richard Henderson
>
> A possible improvement is to push this outside of the search loop and then
> change
>
> }
> - (*info->fprintf_func) (info->stream, "#%8x", insn);
> + info->fprintf_func(info->stream, "");
>return sizeof (insn);
>
> so the byte decode is shared with the rare case of garbage in the insn stream.
Like below?
From: Helge Deller
Subject: [PATCH] disas/hppa: Show hexcode of instruction along with
disassembly
On hppa many instructions can be expressed by different bytecodes.
To be able to debug qemu translation bugs it's therefore necessary to see the
currently executed byte codes without the need to lookup the sequence without
the full executable.
With this patch the instruction byte code is shown beside the disassembly.
Signed-off-by: Helge Deller
diff --git a/disas/hppa.c b/disas/hppa.c
index dcf9a47f34..cce4f4aa37 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1968,6 +1968,10 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
insn = bfd_getb32 (buffer);
+ info->fprintf_func(info->stream, " %02x %02x %02x %02x ",
+(insn >> 24) & 0xff, (insn >> 16) & 0xff,
+(insn >> 8) & 0xff, insn & 0xff);
+
for (i = 0; i < NUMOPCODES; ++i)
{
const struct pa_opcode *opcode = _opcodes[i];
@@ -2826,6 +2830,6 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
return sizeof (insn);
}
}
- (*info->fprintf_func) (info->stream, "#%8x", insn);
+ info->fprintf_func(info->stream, "");
return sizeof (insn);
}
Re: [PATCH-for-8.2] target/nios2: Deprecate the Nios II architecture
On 11/17/23 08:02, Philippe Mathieu-Daudé wrote:
See commit 9ba1caf510 ("MAINTAINERS: Mark the Nios II CPU as orphan"),
last contribution from Chris was in 2012 [1] and Marek in 2018 [2].
[1]
https://lore.kernel.org/qemu-devel/1352607539-10455-2-git-send-email-crwu...@gmail.com/
[2]
https://lore.kernel.org/qemu-devel/805fc7b5-03f0-56d4-abfd-ed010d4fa...@denx.de/
Signed-off-by: Philippe Mathieu-Daudé
Yes please, go for it, from my side:
Acked-by: Marek Vasut
Re: [PATCH v2 2/2] disas/hppa: Show hexcode of instruction along with disassembly
On 11/17/23 02:53, del...@kernel.org wrote:
From: Helge Deller
On hppa many instructions can be expressed by different bytecodes.
To be able to debug qemu translation bugs it's therefore necessary to see the
currently executed byte codes without the need to lookup the sequence without
the full executable.
With this patch the instruction byte code is shown beside the disassembly.
Signed-off-by: Helge Deller
---
disas/hppa.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/disas/hppa.c b/disas/hppa.c
index dcf9a47f34..38fc05acc4 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1979,6 +1979,9 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
if (opcode->arch == pa20w)
continue;
#endif
+ (*info->fprintf_func) (info->stream, " %02x %02x %02x %02x ",
+(insn >> 24) & 0xff, (insn >> 16) & 0xff,
+(insn >> 8) & 0xff, insn & 0xff);
(*info->fprintf_func) (info->stream, "%s", opcode->name);
if (!strchr ("cfCY?-+nHNZFIuv{", opcode->args[0]))
Reviewed-by: Richard Henderson
A possible improvement is to push this outside of the search loop and then
change
}
- (*info->fprintf_func) (info->stream, "#%8x", insn);
+ info->fprintf_func(info->stream, "");
return sizeof (insn);
so the byte decode is shared with the rare case of garbage in the insn stream.
r~
Re: [PATCH v2 1/2] target/hppa: Fix 64-bit SHRPD instruction
On 11/17/23 02:53, del...@kernel.org wrote: From: Helge Deller When shifting the two joined 64-bit registers right, shift the upper 64-bit register to the left and the lower 64-bit register to the right before merging them with OR. Signed-off-by: Helge Deller --- target/hppa/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 17/35] tcg/mips: Support TCG_COND_TST{EQ,NE}
On 11/16/23 23:46, Philippe Mathieu-Daudé wrote: Hi Richard, On 28/10/23 21:45, Richard Henderson wrote: Signed-off-by: Richard Henderson --- tcg/mips/tcg-target.c.inc | 41 +++ 1 file changed, 41 insertions(+) @@ -1053,6 +1071,14 @@ static void tcg_out_setcond2(TCGContext *s, TCGCond cond, TCGReg ret, tcg_out_setcond(s, cond, ret, tmp1, TCG_REG_ZERO); break; + case TCG_COND_TSTEQ: + case TCG_COND_TSTNE: + tcg_out_opc_reg(s, OPC_AND, TCG_TMP0, al, bl); + tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, ah, bh); + tcg_out_opc_reg(s, OPC_OR, ret, TCG_TMP0, TCG_TMP1); + tcg_out_setcond(s, tcg_eqne_cond(cond), ret, tmp1, TCG_REG_ZERO); Where is tcg_eqne_cond() declared? tcg_tst_eqne_cond() is in tcg/tcg-cond.h. This is a rebase error when I renamed it; I have fixed it since. r~
Re: [PATCH-for-8.2] target/nios2: Deprecate the Nios II architecture
On 11/16/23 23:02, Philippe Mathieu-Daudé wrote:
See commit 9ba1caf510 ("MAINTAINERS: Mark the Nios II CPU as orphan"),
last contribution from Chris was in 2012 [1] and Marek in 2018 [2].
[1]
https://lore.kernel.org/qemu-devel/1352607539-10455-2-git-send-email-crwu...@gmail.com/
[2]
https://lore.kernel.org/qemu-devel/805fc7b5-03f0-56d4-abfd-ed010d4fa...@denx.de/
Signed-off-by: Philippe Mathieu-Daudé
---
docs/about/deprecated.rst | 15 +++
hw/nios2/10m50_devboard.c | 1 +
hw/nios2/generic_nommu.c | 1 +
3 files changed, 17 insertions(+)
Reviewed-by: Richard Henderson
r~
Re: [RFC 0/2] vhost-user-test: Add negotiated features check
On Fri, Nov 17, 2023 at 6:29 PM Michael S. Tsirkin wrote: > On Thu, Nov 16, 2023 at 09:01:28AM +0800, Yong Huang wrote: > > ping > > Sit tight pls it's only been a couple of days. > But if you want to, address comments by Markus pls. > > I'm happy to do that, of course, and thank you also for the reminder. :) -- Best regards
Re: [RFC 1/2] qapi/virtio: introduce the "show-bits" argument for x-query-virtio-status
On Thu, Nov 16, 2023 at 10:44 PM Markus Armbruster
wrote:
> Hyman Huang writes:
>
> > This patch allows to display feature and status bits in virtio-status.
> >
> > An optional argument is introduced: show-bits. For example:
> > {"execute": "x-query-virtio-status",
> > "arguments": {"path":
> "/machine/peripheral-anon/device[1]/virtio-backend",
> >"show-bits": true}
> >
> > Features and status bits could be helpful for applications to compare
> > directly. For instance, when an upper application aims to ensure the
> > virtio negotiation correctness between guest, QEMU, and OVS-DPDK, it use
> > the "ovs-vsctl list interface" command to retrieve interface features
> > (in number format) and the QMP command x-query-virtio-status to retrieve
> > vhost-user net device features. If "show-bits" is added, the application
> > can compare the two features directly; No need to encoding the features
> > returned by the QMP command.
> >
> > This patch also serves as a preparation for the next one, which
> implements
> > a vhost-user test case about acked features of vhost-user protocol.
> >
> > Note that since the matching HMP command is typically used for human,
> > leave it unchanged.
> >
> > Signed-off-by: Hyman Huang
> > ---
> > hw/virtio/virtio-hmp-cmds.c | 2 +-
> > hw/virtio/virtio-qmp.c | 21 +++-
> > qapi/virtio.json| 49 ++---
> > 3 files changed, 67 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
> > index 477c97dea2..3774f3d4bf 100644
> > --- a/hw/virtio/virtio-hmp-cmds.c
> > +++ b/hw/virtio/virtio-hmp-cmds.c
> > @@ -108,7 +108,7 @@ void hmp_virtio_status(Monitor *mon, const QDict
> *qdict)
> > {
> > Error *err = NULL;
> > const char *path = qdict_get_try_str(qdict, "path");
> > -VirtioStatus *s = qmp_x_query_virtio_status(path, );
> > +VirtioStatus *s = qmp_x_query_virtio_status(path, false, false,
> );
> >
> > if (err != NULL) {
> > hmp_handle_error(mon, err);
> > diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> > index 1dd96ed20f..2e92bf28ac 100644
> > --- a/hw/virtio/virtio-qmp.c
> > +++ b/hw/virtio/virtio-qmp.c
> > @@ -718,10 +718,15 @@ VirtIODevice *qmp_find_virtio_device(const char
> *path)
> > return VIRTIO_DEVICE(dev);
> > }
> >
> > -VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
> > +VirtioStatus *qmp_x_query_virtio_status(const char *path,
> > +bool has_show_bits,
> > +bool show_bits,
> > +Error **errp)
> > {
> > VirtIODevice *vdev;
> > VirtioStatus *status;
> > +bool display_bits =
> > +has_show_bits ? show_bits : false;
>
> Since !has_show_bits implies !show_bits, you can simply use
> if (show_bits).
>
Ok
>
> >
> > vdev = qmp_find_virtio_device(path);
> > if (vdev == NULL) {
> > @@ -733,6 +738,11 @@ VirtioStatus *qmp_x_query_virtio_status(const char
> *path, Error **errp)
> > status->name = g_strdup(vdev->name);
> > status->device_id = vdev->device_id;
> > status->vhost_started = vdev->vhost_started;
> > +if (display_bits) {
> > +status->guest_features_bits = vdev->guest_features;
> > +status->host_features_bits = vdev->host_features;
> > +status->backend_features_bits = vdev->backend_features;
> > +}
> > status->guest_features = qmp_decode_features(vdev->device_id,
> > vdev->guest_features);
> > status->host_features = qmp_decode_features(vdev->device_id,
> > @@ -753,6 +763,9 @@ VirtioStatus *qmp_x_query_virtio_status(const char
> *path, Error **errp)
> > }
> >
> > status->num_vqs = virtio_get_num_queues(vdev);
> > +if (display_bits) {
> > +status->status_bits = vdev->status;
> > +}
> > status->status = qmp_decode_status(vdev->status);
> > status->isr = vdev->isr;
> > status->queue_sel = vdev->queue_sel;
> > @@ -775,6 +788,12 @@ VirtioStatus *qmp_x_query_virtio_status(const char
> *path, Error **errp)
> > status->vhost_dev->n_tmp_sections = hdev->n_tmp_sections;
> > status->vhost_dev->nvqs = hdev->nvqs;
> > status->vhost_dev->vq_index = hdev->vq_index;
> > +if (display_bits) {
> > +status->vhost_dev->features_bits = hdev->features;
> > +status->vhost_dev->acked_features_bits =
> hdev->acked_features;
> > +status->vhost_dev->backend_features_bits =
> hdev->backend_features;
> > +status->vhost_dev->protocol_features_bits =
> hdev->protocol_features;
> > +}
> > status->vhost_dev->features =
> > qmp_decode_features(vdev->device_id, hdev->features);
> > status->vhost_dev->acked_features =
> > diff --git a/qapi/virtio.json b/qapi/virtio.json
> > index
Re: [PATCH v3 1/8] ppc/pnv: Add pca9552 to powernv10 for PCIe hotplug power control
Well, I was hoping to sweep the pca9554 model under the PowerNV maintainership (like pca9552 is under the BMC aspeed maintainership). I did update the PowerNV list to include it, but perhaps that was presumptuous of me. :-) Well, you are the person who has the most knowledge on both and you have access to HW to check changes ! I would be ok with being added as a reviewer under the PowerNV list, but I wonder if I shouldn't have more opensource experience before becoming a maintainer? TBH, I have no idea what that would entail. For these devices, mostly acking the changes. I don't think anyone will ask you to send PRs. This can be handled through some other tree, PPC or Aspeed. As for patches 3 and 4, it sounds like I should split those changes out from the current patch series so that they can make it into QEMU 8.2. Correct? I don't think this is needed. They can be picked in the series and merged in the ppc tree independently. Thanks, C.
Re: [PULL 0/8] Firmware/edk2 20230918 patches
On 2023/11/13 20:09, Gerd Hoffmann wrote: Hi, This apparently broke EDK2 for AArch64. I tried the following command: build/qemu-system-aarch64 -drive file=build/pc-bios/edk2-aarch64-code.fd,format=raw,if=pflash,readonly=on -M virt -cpu max -nographic -cdrom Fedora-Silverblue-ostree-aarch64-37-1.7.iso https://bugzilla.redhat.com/show_bug.cgi?id=2113005 tl:dr: shim is broken, and recent edk2 starting to expose EFI_MEMORY_ATTRIBUTE_PROTOCOL makes the bug visible (without the protocol the buggy code path is never taken). take care, Gerd That's unfortunate. Thanks for the info. Regards, Akihiko Odaki
Re: [PATCH-for-8.2] target/nios2: Deprecate the Nios II architecture
On 11/17/23 01:46, Thomas Huth wrote: Being orphan for so long in QEMU, I guess it makes sense to mark it as deprecated here now. We can still reconsider if a new maintainer shows up... Reviewed-by: Thomas Huth I agree, but I'd be surprised if anybody steps forward, since Intel has pretty much dropped all support for the nios2 architecture now (their current FPGA products based on risc-v). At this point it looks very much like the upcoming GCC 14 release will be the last that includes support for this target. -Sandra
Re: [PATCH v6 18/21] vfio: Make VFIOContainerBase poiner parameter const in VFIOIOMMUOps callbacks
On 11/14/23 11:09, Zhenzhong Duan wrote:
> Some of the callbacks in VFIOIOMMUOps pass VFIOContainerBase poiner,
> those callbacks only need read access to the sub object of VFIOContainerBase.
> So make VFIOContainerBase, VFIOContainer and VFIOIOMMUFDContainer as const
> in these callbacks.
>
> Local functions called by those callbacks also need same changes to avoid
> build error.
>
> Suggested-by: Cédric Le Goater
> Signed-off-by: Zhenzhong Duan
Reviewed-by: Eric Auger
Thanks
Eric
> ---
> include/hw/vfio/vfio-common.h | 12 ++
> include/hw/vfio/vfio-container-base.h | 12 ++
> hw/vfio/common.c | 9 +++
> hw/vfio/container-base.c | 2 +-
> hw/vfio/container.c | 34 ++-
> hw/vfio/iommufd.c | 8 +++
> 6 files changed, 42 insertions(+), 35 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 567e5f7bea..7954531d05 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -244,13 +244,15 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error
> **errp);
> void vfio_migration_exit(VFIODevice *vbasedev);
>
> int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
> -bool vfio_devices_all_running_and_mig_active(VFIOContainerBase *bcontainer);
> -bool vfio_devices_all_device_dirty_tracking(VFIOContainerBase *bcontainer);
> -int vfio_devices_query_dirty_bitmap(VFIOContainerBase *bcontainer,
> +bool
> +vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer);
> +bool
> +vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer);
> +int vfio_devices_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
> VFIOBitmap *vbmap, hwaddr iova,
> hwaddr size);
> -int vfio_get_dirty_bitmap(VFIOContainerBase *bcontainer, uint64_t iova,
> - uint64_t size, ram_addr_t ram_addr);
> +int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
> + uint64_t size, ram_addr_t ram_addr);
>
> int vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
> void vfio_device_set_fd(VFIODevice *vbasedev, const char *str, Error **errp);
> diff --git a/include/hw/vfio/vfio-container-base.h
> b/include/hw/vfio/vfio-container-base.h
> index 45bb19c767..2ae297ccda 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -82,7 +82,7 @@ void vfio_container_del_section_window(VFIOContainerBase
> *bcontainer,
> MemoryRegionSection *section);
> int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
> bool start);
> -int vfio_container_query_dirty_bitmap(VFIOContainerBase *bcontainer,
> +int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
>VFIOBitmap *vbmap,
>hwaddr iova, hwaddr size);
>
> @@ -93,18 +93,20 @@ void vfio_container_destroy(VFIOContainerBase
> *bcontainer);
>
> struct VFIOIOMMUOps {
> /* basic feature */
> -int (*dma_map)(VFIOContainerBase *bcontainer,
> +int (*dma_map)(const VFIOContainerBase *bcontainer,
> hwaddr iova, ram_addr_t size,
> void *vaddr, bool readonly);
> -int (*dma_unmap)(VFIOContainerBase *bcontainer,
> +int (*dma_unmap)(const VFIOContainerBase *bcontainer,
> hwaddr iova, ram_addr_t size,
> IOMMUTLBEntry *iotlb);
> int (*attach_device)(const char *name, VFIODevice *vbasedev,
> AddressSpace *as, Error **errp);
> void (*detach_device)(VFIODevice *vbasedev);
> /* migration feature */
> -int (*set_dirty_page_tracking)(VFIOContainerBase *bcontainer, bool
> start);
> -int (*query_dirty_bitmap)(VFIOContainerBase *bcontainer, VFIOBitmap
> *vbmap,
> +int (*set_dirty_page_tracking)(const VFIOContainerBase *bcontainer,
> + bool start);
> +int (*query_dirty_bitmap)(const VFIOContainerBase *bcontainer,
> + VFIOBitmap *vbmap,
>hwaddr iova, hwaddr size);
> /* PCI specific */
> int (*pci_hot_reset)(VFIODevice *vbasedev, bool single);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6569732b7a..08a3e57672 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -204,7 +204,7 @@ static bool
> vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
> return true;
> }
>
> -bool vfio_devices_all_device_dirty_tracking(VFIOContainerBase *bcontainer)
> +bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase
> *bcontainer)
> {
> VFIODevice *vbasedev;
>
> @@ -221,7
Re: [PATCH v6 12/21] vfio/platform: Allow the selection of a given iommu backend
Hi Zhenzhong,
On 11/14/23 11:09, Zhenzhong Duan wrote:
> Now we support two types of iommu backends, let's add the capability
> to select one of them. This depends on whether an iommufd object has
> been linked with the vfio-platform device:
>
> If the user wants to use the legacy backend, it shall not
> link the vfio-platform device with any iommufd object:
>
> -device vfio-platform,host=XXX
>
> This is called the legacy mode/backend.
>
> If the user wants to use the iommufd backend (/dev/iommu) it
> shall pass an iommufd object id in the vfio-platform device options:
>
> -object iommufd,id=iommufd0
> -device vfio-platform,host=XXX,iommufd=iommufd0
>
> Suggested-by: Alex Williamson
> Signed-off-by: Zhenzhong Duan
Reviewed-by: Eric Auger
Eric
> ---
> v6: Move #include "sysemu/iommufd.h" in platform.c
>
> hw/vfio/platform.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 8e3d4ac458..98ae4bc655 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -15,11 +15,13 @@
> */
>
> #include "qemu/osdep.h"
> +#include CONFIG_DEVICES /* CONFIG_IOMMUFD */
> #include "qapi/error.h"
> #include
> #include
>
> #include "hw/vfio/vfio-platform.h"
> +#include "sysemu/iommufd.h"
> #include "migration/vmstate.h"
> #include "qemu/error-report.h"
> #include "qemu/lockable.h"
> @@ -649,6 +651,10 @@ static Property vfio_platform_dev_properties[] = {
> DEFINE_PROP_UINT32("mmap-timeout-ms", VFIOPlatformDevice,
> mmap_timeout, 1100),
> DEFINE_PROP_BOOL("x-irqfd", VFIOPlatformDevice, irqfd_allowed, true),
> +#ifdef CONFIG_IOMMUFD
> +DEFINE_PROP_LINK("iommufd", VFIOPlatformDevice, vbasedev.iommufd,
> + TYPE_IOMMUFD_BACKEND, IOMMUFDBackend *),
> +#endif
> DEFINE_PROP_END_OF_LIST(),
> };
>
[PATCH for-8.2 3/3] ui/console: fix default VC when there are no display
From: Marc-André Lureau
When display is "none", we may still have remote displays (I think it
would be simpler if VNC/Spice were regular display btw). Return the
default VC then, and set them up to fix a regression when using remote
display and it used the TTY instead.
Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC")
Reported-by: German Maglione
Signed-off-by: Marc-André Lureau
---
ui/console.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/ui/console.c b/ui/console.c
index 8e688d3569..7db921e3b7 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1679,19 +1679,17 @@ void qemu_display_init(DisplayState *ds, DisplayOptions
*opts)
const char *qemu_display_get_vc(DisplayOptions *opts)
{
-assert(opts->type < DISPLAY_TYPE__MAX);
-if (opts->type == DISPLAY_TYPE_NONE) {
-return NULL;
-}
-assert(dpys[opts->type] != NULL);
-if (dpys[opts->type]->vc) {
-return dpys[opts->type]->vc;
-} else {
#ifdef CONFIG_PIXMAN
-return "vc:80Cx24C";
+const char *vc = "vc:80Cx24C";
+#else
+const char *vc = NULL;
#endif
+
+assert(opts->type < DISPLAY_TYPE__MAX);
+if (dpys[opts->type] && dpys[opts->type]->vc) {
+vc = dpys[opts->type]->vc;
}
-return NULL;
+return vc;
}
void qemu_display_help(void)
--
2.41.0
[PATCH for-8.2 2/3] ui: use "vc" chardev for dbus, gtk & spice-app
From: Marc-André Lureau
Those display have their own implementation of "vc" chardev, which
doesn't use pixman. They also don't implement the width/height/cols/rows
options, so qemu_display_get_vc() should return a compatible argument.
This patch was meant to be with the pixman series, when the "vc" field
was introduced. It fixes a regression where VC are created on the
tty (or null) instead of the display own "vc" implementation.
Signed-off-by: Marc-André Lureau
---
ui/dbus.c | 1 +
ui/gtk.c | 1 +
ui/spice-app.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/ui/dbus.c b/ui/dbus.c
index 866467ad2e..e08b5de064 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -518,6 +518,7 @@ static QemuDisplay qemu_display_dbus = {
.type = DISPLAY_TYPE_DBUS,
.early_init = early_dbus_init,
.init = dbus_init,
+.vc = "vc",
};
static void register_dbus(void)
diff --git a/ui/gtk.c b/ui/gtk.c
index be047a41ad..810d7fc796 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2534,6 +2534,7 @@ static QemuDisplay qemu_display_gtk = {
.type = DISPLAY_TYPE_GTK,
.early_init = early_gtk_display_init,
.init = gtk_display_init,
+.vc = "vc",
};
static void register_gtk(void)
diff --git a/ui/spice-app.c b/ui/spice-app.c
index 405fb7f9f5..a10b4a58fe 100644
--- a/ui/spice-app.c
+++ b/ui/spice-app.c
@@ -220,6 +220,7 @@ static QemuDisplay qemu_display_spice_app = {
.type = DISPLAY_TYPE_SPICE_APP,
.early_init = spice_app_display_early_init,
.init = spice_app_display_init,
+.vc = "vc",
};
static void register_spice_app(void)
--
2.41.0
[PATCH for-8.2 1/3] vl: revert behaviour for -display none
From: Marc-André Lureau
Commit 1bec1cc0d ("ui/console: allow to override the default VC") changed
the behaviour of the "-display none" option, so that it now creates a
QEMU monitor on the terminal. "-display none" should not be tangled up
with whether we create a monitor or a serial terminal; it should purely
and only disable the graphical window. Changing its behaviour like this
breaks command lines which, for example, use semihosting for their
output and don't want a graphical window, as they now get a monitor they
never asked for.
It also breaks the command line we document for Xen in
docs/system/i386/xen.html:
$ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split \
-display none -chardev stdio,mux=on,id=char0,signal=off -mon char0 \
-device xen-console,chardev=char0 -drive file=${GUEST_IMAGE},if=xen
qemu-system-x86_64: cannot use stdio by multiple character devices
qemu-system-x86_64: could not connect serial device to character backend
'stdio'
When qemu is compiled without PIXMAN, by default the serials aren't
muxed with the monitor anymore on stdio. The serials are redirected to
"null" instead, and the monitor isn't set up.
Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC")
Signed-off-by: Marc-André Lureau
---
system/vl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/system/vl.c b/system/vl.c
index 5af7ced2a1..14bf0cf0bf 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void)
}
}
-if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) {
+if (nographic) {
if (default_parallel) {
add_device_config(DEV_PARALLEL, "null");
}
--
2.41.0
[PATCH for-8.2 0/3] UI: fix default VC regressions
From: Marc-André Lureau Hi, There are a few annoying regressions with the default VCs introduced with the pixman series. The "vl: revert behaviour for -display none" change solves most of the issues. Another one is hit when using remote displays, and VCs are not created as they used to, see: "ui/console: fix default VC when there are no display". Finally, "ui: use "vc" chardev for dbus, gtk & spice-app" was meant to be included in the pixman series and also brings back default VCs creation. Marc-André Lureau (3): vl: revert behaviour for -display none ui: use "vc" chardev for dbus, gtk & spice-app ui/console: fix default VC when there are no display system/vl.c| 2 +- ui/console.c | 18 -- ui/dbus.c | 1 + ui/gtk.c | 1 + ui/spice-app.c | 1 + 5 files changed, 12 insertions(+), 11 deletions(-) -- 2.41.0
Re: [PATCH] docs/devel: Add VFIO iommufd backend documentation
On 11/17/23 13:58, Cédric Le Goater wrote: On 11/17/23 10:35, Zhenzhong Duan wrote: Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan The content looks good but it lacks formatting. Please try to generate the docs. Please check my vfio-8.2 branch. Thanks, C.
Re: [PATCH v3 0/4] ide: implement simple legacy/native mode switching for PCI IDE controllers
On Thu, 16 Nov 2023, BALATON Zoltan wrote: On Thu, 16 Nov 2023, Kevin Wolf wrote: Am 16.11.2023 um 11:33 hat Mark Cave-Ayland geschrieben: This series adds a simple implementation of legacy/native mode switching for PCI IDE controllers and updates the via-ide device to use it. The approach I take here is to add a new pci_ide_update_mode() function which handles management of the PCI BARs and legacy IDE ioports for each mode to avoid exposing details of the internal logic to individual PCI IDE controllers. As noted in [1] this is extracted from a local WIP branch I have which contains further work in this area. However for the moment I've kept it simple (and restricted it to the via-ide device) which is good enough for Zoltan's PPC images whilst paving the way for future improvements after 8.2. Signed-off-by: Mark Cave-Ayland [1] https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg05403.html v3: - Rebase onto master - Move ide_portio_list[] and ide_portio_list2[] to IDE core to prevent duplication in hw/ide/pci.c - Don't zero BARs when switching from native mode to legacy mode, instead always force them to read zero as suggested in the PCI IDE specification (note: this also appears to fix the fuloong2e machine booting from IDE) - Add comments in pci_ide_update_mode() suggested by Kevin - Drop the existing R-B and T-B tags: whilst this passes my local tests, the behaviour around zero BARs feels different enough here Thanks, applied to the block branch. I feel a bit left out of this conversation... Did Google or some other spam filter decide again to filter my messages so you did not see them at all? Could you confitm that you've got my previous two replies in this thread so I know I'm not sending comments to /dev/null please? Looks like there's some issue with these mails. They appear in the list archive but maybe not in people's mailboxes? Did any of you got this message and previous ones I've sent? https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg03180.html https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg03983.html Regards, BALATON Zoltan
[PATCH] vl: add missing display_remote++
From: Marc-André Lureau
We should also consider -display vnc= as setting up a remote display,
and not attempt to add another default one.
The display_remote++ in qemu_setup_display() isn't necessary at this
point, but is there for completeness and further usages of the variable.
Signed-off-by: Marc-André Lureau
---
system/vl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/system/vl.c b/system/vl.c
index 5af7ced2a1..f95ae77b5a 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1110,6 +1110,7 @@ static void parse_display(const char *p)
*/
if (*opts == '=') {
vnc_parse(opts + 1);
+display_remote++;
} else {
error_report("VNC requires a display argument vnc=");
exit(1);
@@ -1359,6 +1360,7 @@ static void qemu_setup_display(void)
dpy.type = DISPLAY_TYPE_NONE;
#if defined(CONFIG_VNC)
vnc_parse("localhost:0,to=99,id=default");
+display_remote++;
#endif
}
}
--
2.41.0
Re: [PATCH 03/16] hw/uefi: add include/hw/uefi/var-service.h
On 11/15/23 16:12, Gerd Hoffmann wrote:
> Add state structs and function declarations for the uefi-vars device.
>
> Signed-off-by: Gerd Hoffmann
> ---
> include/hw/uefi/var-service.h | 119 ++
> 1 file changed, 119 insertions(+)
> create mode 100644 include/hw/uefi/var-service.h
>
> diff --git a/include/hw/uefi/var-service.h b/include/hw/uefi/var-service.h
> new file mode 100644
> index ..2b8d3052e59f
> --- /dev/null
> +++ b/include/hw/uefi/var-service.h
> @@ -0,0 +1,119 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * uefi-vars device - state struct and function prototypes
> + */
> +#ifndef QEMU_UEFI_VAR_SERVICE_H
> +#define QEMU_UEFI_VAR_SERVICE_H
> +
> +#include "qemu/uuid.h"
> +#include "qemu/queue.h"
> +
> +#include "hw/uefi/var-service-edk2.h"
> +
> +#define MAX_BUFFER_SIZE (64 * 1024)
> +
> +typedef struct uefi_variable uefi_variable;
> +typedef struct uefi_var_policy uefi_var_policy;
> +typedef struct uefi_vars_state uefi_vars_state;
> +
> +struct uefi_variable {
> +QemuUUID guid;
> +uint16_t *name;
> +uint32_t name_size;
> +uint32_t attributes;
> +void *data;
> +uint32_t data_size;
> +QTAILQ_ENTRY(uefi_variable) next;
> +};
> +
> +struct uefi_var_policy {
> +variable_policy_entry *entry;
> +uint32_t entry_size;
> +uint16_t *name;
> +uint32_t name_size;
> +uint32_t hashmarks;
> +QTAILQ_ENTRY(uefi_var_policy) next;
> +};
- I wonder if the size fields should be size_t. uint32_t is not wrong
either; we'll just have to be careful when doing comparisons etc.
- care to explain (in a comment) hashmarks? I think it's related to the
wildcard policy stuff, but a hint would be appreciated.
> +
> +struct uefi_vars_state {
> +MemoryRegion mr;
> +uint16_t sts;
> +uint32_t buf_size;
> +uint32_t buf_addr_lo;
> +uint32_t buf_addr_hi;
spelling out endianness here would be useful IMO
> +uint8_t *buffer;
> +QTAILQ_HEAD(, uefi_variable) variables;
> +QTAILQ_HEAD(, uefi_var_policy)var_policies;
> +
> +/* boot phases */
> +bool end_of_dxe;
> +bool ready_to_boot;
> +bool exit_boot_service;
There are some variations of the 8 possible that don't make sense. at
the same time, a single enum could be too limiting. depends on what the
code will do with these.
> +bool policy_locked;
> +
> +/* storage accounting */
> +uint64_t max_storage;
> +uint64_t used_storage;
> +
> +char *jsonfile;
> +int jsonfd;
> +};
> +
> +/* vars-service-guid.c */
> +extern QemuUUID EfiGlobalVariable;
> +extern QemuUUID EfiImageSecurityDatabase;
> +extern QemuUUID EfiCustomModeEnable;
> +extern QemuUUID EfiSecureBootEnableDisable;
> +extern QemuUUID EfiSmmVariableProtocolGuid;
> +extern QemuUUID VarCheckPolicyLibMmiHandlerGuid;
> +extern QemuUUID EfiEndOfDxeEventGroupGuid;
> +extern QemuUUID EfiEventReadyToBootGuid;
> +extern QemuUUID EfiEventExitBootServicesGuid;
the spelling of these names appears a bit questionable:
- camelcase is idiomatic in edk2, but (I think?) not in QEMU, for variables
- the "Guid" suffix is inconsistently used / carried over from edk2
> +
> +/* vars-service-core.c */
> +extern const VMStateDescription vmstate_uefi_vars;
> +size_t uefi_strlen(const uint16_t *str, size_t len);
> +gboolean uefi_str_equal(const uint16_t *a, size_t alen,
> +const uint16_t *b, size_t blen);
> +char *uefi_ucs2_to_ascii(const uint16_t *ucs2, uint64_t ucs2_size);
> +void uefi_trace_variable(const char *action, QemuUUID guid,
> + const uint16_t *name, uint64_t name_size);
> +void uefi_trace_status(const char *action, efi_status status);
> +void uefi_vars_init(Object *obj, uefi_vars_state *uv);
> +void uefi_vars_realize(uefi_vars_state *uv, Error **errp);
> +void uefi_vars_hard_reset(uefi_vars_state *uv);
> +
> +/* vars-service-json.c */
> +void uefi_vars_json_init(uefi_vars_state *uv, Error **errp);
> +void uefi_vars_json_save(uefi_vars_state *uv);
> +void uefi_vars_json_load(uefi_vars_state *uv, Error **errp);
> +
> +/* vars-service-vars.c */
> +extern const VMStateDescription vmstate_uefi_variable;
> +uefi_variable *uefi_vars_find_variable(uefi_vars_state *uv, QemuUUID guid,
> + const uint16_t *name,
> +
Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
On 11/17/23 14:29, Eric Auger wrote:
Hi Cédric,
On 11/17/23 12:39, Duan, Zhenzhong wrote:
Hi Cédric,
-Original Message-
From: Cédric Le Goater
Sent: Friday, November 17, 2023 7:10 PM
Subject: Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
Hello,
+int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id,
hwaddr iova,
+ram_addr_t size, void *vaddr, bool readonly)
+{
+int ret, fd = be->fd;
+struct iommu_ioas_map map = {
+.size = sizeof(map),
+.flags = IOMMU_IOAS_MAP_READABLE |
+ IOMMU_IOAS_MAP_FIXED_IOVA,
+.ioas_id = ioas_id,
+.__reserved = 0,
+.user_va = (uintptr_t)vaddr,
+.iova = iova,
+.length = size,
+};
+
+if (!readonly) {
+map.flags |= IOMMU_IOAS_MAP_WRITEABLE;
+}
+
+ret = ioctl(fd, IOMMU_IOAS_MAP, );
+trace_iommufd_backend_map_dma(fd, ioas_id, iova, size,
+ vaddr, readonly, ret);
+if (ret) {
+ret = -errno;
+error_report("IOMMU_IOAS_MAP failed: %m");
+}
+return ret;
+}
When using a UEFI guest, QEMU reports errors when mapping regions
in the top PCI space :
iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38001000
size=0x3000 addr=0x7fce2c28b000 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
0x38001000, 0x3000, 0x7fce2c28b000) = -22 (Invalid argument)
iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38004000
size=0x4000 addr=0x7fce2c98 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
0x38004000, 0x4000, 0x7fce2c98) = -22 (Invalid argument)
This is because IOMMUFD reserved IOVAs areas are :
[ fee0 - feef ]
[ 80 - ] (39 bits address space)
which were allocated when the device was initially attached.
The topology is basic. Something is wrong.
Thanks for your report. This looks a hardware limit of
host IOMMU address width(39) < guest physical address width.
A similar issue with a fix submitted below, ccing related people.
https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg02937.html
It looks the fix will not work for hotplug.
Or below qemu cmdline may help:
"-cpu host,host-phys-bits-limit=39"
don't you have the same issue with legacy VFIO code, you should?
I tend to be lazy and use seabios for guests on the command line.
I do see the error with legacy VFIO and uefi.
However, with the address space size work-around and iommufd, the
error is different, an EFAULT now. Some page pinning issue it seems.
Thanks,
C.
Re: [PATCH v2] migration: free 'saddr' since be no longer used
On Fri, Nov 17, 2023 at 10:51:18AM +0800, Zongmin Zhou wrote: > > As Peter said, putting a comment why we don't use > > qapi_free_SocketAddress() will be a good idea. > > I have put some comments on patch v2 to explain Normally we use "comment" to represent direct comment in the code. You explained it in the "commit message". :) That explanation is good enough to me, you can add a summary comment in the code too. Something like: /* Don't free the objects inside; their ownership moved to "addr" */ -- Peter Xu
Re: [PATCH v6 09/21] vfio/iommufd: Enable pci hot reset through iommufd cdev interface
On 11/14/23 11:09, Zhenzhong Duan wrote:
> Add a new callback iommufd_cdev_pci_hot_reset to do iommufd specific
> check and reset operation.
nit: Implement the newly introduced pci_hot_reset callback?
>
> Signed-off-by: Zhenzhong Duan
> ---
> v6: pci_hot_reset return -errno if fails
>
> hw/vfio/iommufd.c| 145 +++
> hw/vfio/trace-events | 1 +
> 2 files changed, 146 insertions(+)
>
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index e5bf528e89..3eec428162 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -24,6 +24,7 @@
> #include "sysemu/reset.h"
> #include "qemu/cutils.h"
> #include "qemu/chardev_open.h"
> +#include "pci.h"
>
> static int iommufd_cdev_map(VFIOContainerBase *bcontainer, hwaddr iova,
> ram_addr_t size, void *vaddr, bool readonly)
> @@ -473,9 +474,153 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
> close(vbasedev->fd);
> }
>
> +static VFIODevice *iommufd_cdev_pci_find_by_devid(__u32 devid)
> +{
> +VFIODevice *vbasedev_iter;
> +
> +QLIST_FOREACH(vbasedev_iter, _device_list, global_next) {
> +if (vbasedev_iter->bcontainer->ops != _iommufd_ops) {
> +continue;
> +}
> +if (devid == vbasedev_iter->devid) {
> +return vbasedev_iter;
> +}
> +}
> +return NULL;
> +}
> +
> +static int iommufd_cdev_pci_hot_reset(VFIODevice *vbasedev, bool single)
> +{
> +VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
> +struct vfio_pci_hot_reset_info *info = NULL;
> +struct vfio_pci_dependent_device *devices;
> +struct vfio_pci_hot_reset *reset;
> +int ret, i;
> +bool multi = false;
> +
> +trace_vfio_pci_hot_reset(vdev->vbasedev.name, single ? "one" : "multi");
> +
> +if (!single) {
> +vfio_pci_pre_reset(vdev);
> +}
> +vdev->vbasedev.needs_reset = false;
> +
> +ret = vfio_pci_get_pci_hot_reset_info(vdev, );
> +
> +if (ret) {
> +goto out_single;
> +}
> +
> +assert(info->flags & VFIO_PCI_HOT_RESET_FLAG_DEV_ID);
> +
> +devices = >devices[0];
> +
> +if (!(info->flags & VFIO_PCI_HOT_RESET_FLAG_DEV_ID_OWNED)) {
> +if (!vdev->has_pm_reset) {
> +for (i = 0; i < info->count; i++) {
> +if (devices[i].devid == VFIO_PCI_DEVID_NOT_OWNED) {
> +error_report("vfio: Cannot reset device %s, "
> + "depends on device %04x:%02x:%02x.%x "
> + "which is not owned.",
> + vdev->vbasedev.name, devices[i].segment,
> + devices[i].bus, PCI_SLOT(devices[i].devfn),
> + PCI_FUNC(devices[i].devfn));
> +}
> +}
> +}
> +ret = -EPERM;
> +goto out_single;
> +}
> +
> +trace_vfio_pci_hot_reset_has_dep_devices(vdev->vbasedev.name);
> +
> +for (i = 0; i < info->count; i++) {
> +VFIOPCIDevice *tmp;
> +VFIODevice *vbasedev_iter;
> +
> +trace_iommufd_cdev_pci_hot_reset_dep_devices(devices[i].segment,
> + devices[i].bus,
> +
> PCI_SLOT(devices[i].devfn),
> +
> PCI_FUNC(devices[i].devfn),
> + devices[i].devid);
> +
> +/*
> + * If a VFIO cdev device is resettable, all the dependent devices
> + * are either bound to same iommufd or within same iommu_groups as
> + * one of the iommufd bound devices.
> + */
> +assert(devices[i].devid != VFIO_PCI_DEVID_NOT_OWNED);
> +
> +if (devices[i].devid == vdev->vbasedev.devid ||
> +devices[i].devid == VFIO_PCI_DEVID_OWNED) {
> +continue;
> +}
> +
> +vbasedev_iter = iommufd_cdev_pci_find_by_devid(devices[i].devid);
> +if (!vbasedev_iter || !vbasedev_iter->dev->realized ||
> +vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
> +continue;
> +}
> +tmp = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
> +if (single) {
> +ret = -EINVAL;
> +goto out_single;
> +}
> +vfio_pci_pre_reset(tmp);
> +tmp->vbasedev.needs_reset = false;
> +multi = true;
> +}
> +
> +if (!single && !multi) {
> +ret = -EINVAL;
> +goto out_single;
> +}
> +
> +/* Use zero length array for hot reset with iommufd backend */
> +reset = g_malloc0(sizeof(*reset));
> +reset->argsz = sizeof(*reset);
> +
> + /* Bus reset! */
> +ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
> +g_free(reset);
> +if (ret) {
> +ret = -errno;
> +}
> +
> +
Re: [PATCH v3 5/5] qom/object: Limit type names to alphanumerical and some few special characters
On 17/11/2023 13.25, Philippe Mathieu-Daudé wrote:
On 17/11/23 12:44, Thomas Huth wrote:
QOM names currently don't have any enforced naming rules. This
can be problematic, e.g. when they are used on the command line
for the "-device" option (where the comma is used to separate
properties). To avoid that such problematic type names come in
again, let's restrict the set of acceptable characters during the
type registration.
Ideally, we'd apply here the same rules as for QAPI, i.e. all type
names should begin with a letter, and contain only ASCII letters,
digits, hyphen, and underscore. However, we already have so many
pre-existing types like:
486-x86_64-cpu
cfi.pflash01
power5+_v2.1-spapr-cpu-core
virt-2.6-machine
pc-i440fx-3.0-machine
... so that we have to allow "." and "+" for now, too. While the
dot is used in a lot of places, the "+" can fortunately be limited
to two classes of legacy names ("power" and "Sun-UltraSparc" CPUs).
We also cannot enforce the rule that names must start with a letter
yet, since there are lot of types that start with a digit. Still,
at least limiting the first characters to the alphanumerical range
should be way better than nothing.
Signed-off-by: Thomas Huth
---
qom/object.c | 41 +
1 file changed, 41 insertions(+)
diff --git a/qom/object.c b/qom/object.c
index 95c0dc8285..654e1afaf2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -138,9 +138,50 @@ static TypeImpl *type_new(const TypeInfo *info)
return ti;
}
+static bool type_name_is_valid(const char *name)
+{
+ const int slen = strlen(name);
+ int plen;
+
+ g_assert(slen > 1);
+
+ /*
+ * Ideally, the name should start with a letter - however, we've got
+ * too many names starting with a digit already, so allow digits here,
+ * too (except '0' which is not used yet)
+ */
+ if (!g_ascii_isalnum(name[0]) || name[0] == '0') {
+ return false;
+ }
+
+ plen = strspn(name, "abcdefghijklmnopqrstuvwxyz"
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+ "0123456789-_.");
+
+ /* Allow some legacy names with '+' in it for compatibility reasons */
+ if (name[plen] == '+') {
+ if (plen == 6 && g_str_has_prefix(name, "power")) {
+ /* Allow "power5+" and "power7+" CPU names*/
+ return true;
+ }
+ if (plen >= 17 && g_str_has_prefix(name, "Sun-UltraSparc-I")) {
+ /* Allow "Sun-UltraSparc-IV+" and "Sun-UltraSparc-IIIi+" */
+ return true;
+ }
+ }
+
+ return plen == slen;
+}
+
static TypeImpl *type_register_internal(const TypeInfo *info)
{
TypeImpl *ti;
+
+ if (!type_name_is_valid(info->name)) {
+ fprintf(stderr, "Registering '%s' with illegal type name\n",
info->name);
Shouldn't we use error_report() instead of fprintf()? Regardless,
It doesn't work here yet - the type registration happens so early that we
cannot use error_report() here yet.
Reviewed-by: Philippe Mathieu-Daudé
Thanks!
Thomas
Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
Hi Cédric,
On 11/17/23 12:39, Duan, Zhenzhong wrote:
> Hi Cédric,
>
>> -Original Message-
>> From: Cédric Le Goater
>> Sent: Friday, November 17, 2023 7:10 PM
>> Subject: Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
>>
>> Hello,
>>
>>> +int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id,
>> hwaddr iova,
>>> +ram_addr_t size, void *vaddr, bool readonly)
>>> +{
>>> +int ret, fd = be->fd;
>>> +struct iommu_ioas_map map = {
>>> +.size = sizeof(map),
>>> +.flags = IOMMU_IOAS_MAP_READABLE |
>>> + IOMMU_IOAS_MAP_FIXED_IOVA,
>>> +.ioas_id = ioas_id,
>>> +.__reserved = 0,
>>> +.user_va = (uintptr_t)vaddr,
>>> +.iova = iova,
>>> +.length = size,
>>> +};
>>> +
>>> +if (!readonly) {
>>> +map.flags |= IOMMU_IOAS_MAP_WRITEABLE;
>>> +}
>>> +
>>> +ret = ioctl(fd, IOMMU_IOAS_MAP, );
>>> +trace_iommufd_backend_map_dma(fd, ioas_id, iova, size,
>>> + vaddr, readonly, ret);
>>> +if (ret) {
>>> +ret = -errno;
>>> +error_report("IOMMU_IOAS_MAP failed: %m");
>>> +}
>>> +return ret;
>>> +}
>> When using a UEFI guest, QEMU reports errors when mapping regions
>> in the top PCI space :
>>
>> iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38001000
>> size=0x3000 addr=0x7fce2c28b000 readonly=0 (-1)
>> qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
>> qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
>> 0x38001000, 0x3000, 0x7fce2c28b000) = -22 (Invalid argument)
>>
>> iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38004000
>> size=0x4000 addr=0x7fce2c98 readonly=0 (-1)
>> qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
>> qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
>> 0x38004000, 0x4000, 0x7fce2c98) = -22 (Invalid argument)
>>
>> This is because IOMMUFD reserved IOVAs areas are :
>>
>> [ fee0 - feef ]
>> [ 80 - ] (39 bits address space)
>>
>> which were allocated when the device was initially attached.
>> The topology is basic. Something is wrong.
>
> Thanks for your report. This looks a hardware limit of
> host IOMMU address width(39) < guest physical address width.
>
> A similar issue with a fix submitted below, ccing related people.
> https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg02937.html
> It looks the fix will not work for hotplug.
>
> Or below qemu cmdline may help:
> "-cpu host,host-phys-bits-limit=39"
don't you have the same issue with legacy VFIO code, you should?
Eric
>
> Thanks
> Zhenzhong
>
Re: [PATCH] docs/devel: Add VFIO iommufd backend documentation
On 11/17/23 10:35, Zhenzhong Duan wrote: Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan The content looks good but it lacks formatting. Please try to generate the docs. Thanks, C. --- MAINTAINERS| 1 + docs/devel/index-internals.rst | 1 + docs/devel/vfio-iommufd.rst| 115 + 3 files changed, 117 insertions(+) create mode 100644 docs/devel/vfio-iommufd.rst diff --git a/MAINTAINERS b/MAINTAINERS index d86ba56a49..07990456ed 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2175,6 +2175,7 @@ F: backends/iommufd.c F: include/sysemu/iommufd.h F: include/qemu/chardev_open.h F: util/chardev_open.c +F: docs/devel/vfio-iommufd.rst vhost M: Michael S. Tsirkin diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst index 6f81df92bc..3def4a138b 100644 --- a/docs/devel/index-internals.rst +++ b/docs/devel/index-internals.rst @@ -18,5 +18,6 @@ Details about QEMU's various subsystems including how to add features to them. s390-dasd-ipl tracing vfio-migration + vfio-iommufd writing-monitor-commands virtio-backends diff --git a/docs/devel/vfio-iommufd.rst b/docs/devel/vfio-iommufd.rst new file mode 100644 index 00..59804a7f26 --- /dev/null +++ b/docs/devel/vfio-iommufd.rst @@ -0,0 +1,115 @@ +=== +IOMMUFD BACKEND usage with VFIO +=== + +(Same meaning for backend/container/BE) + +With the introduction of iommufd, the Linux kernel provides a generic +interface for user space drivers to propagate their DMA mappings to kernel +for assigned devices. While the legacy kernel interface is group-centric, +the new iommufd interface is device-centric, relying on device fd and iommufd. + +To support both interfaces in the QEMU VFIO device, introduce a base container +to abstract the common part of VFIO legacy and iommufd container. So that the +generic VFIO code can use either container. + +The base container implements generic functions such as memory_listener and +address space management whereas the derived container implements callbacks +specific to either legacy or iommufd. Each container has its own way to setup +secure context and dma management interface. The below diagram shows how it +looks like with both containers. + +VFIO AddressSpace/Memory ++---+ +--+ +-+ +-+ +| pci | | platform | | ap | | ccw | ++---+---+ ++-+ +--+--+ +--+--+ +--+ +| | ||| AddressSpace | +| | ||++-+ ++---V---V---VV+ / +| VFIOAddressSpace | <+ +| | | MemoryListener +|VFIOContainerBase list | ++---+++ +|| +|| ++---V--++V--+ +| iommufd||vfio legacy| +| container || container | ++---+--+++--+ +|| +| /dev/iommu | /dev/vfio/vfio +| /dev/vfio/devices/vfioX| /dev/vfio/$group_id +Userspace || +++=== +Kernel | device fd | ++---+| group/container fd +| (BIND_IOMMUFD || (SET_CONTAINER/SET_IOMMU) +| ATTACH_IOAS) || device fd +| || +| +---VV-+ +iommufd | |vfio | +(map/unmap | +-++---+ +ioas_copy) | || map/unmap +| || + +--V--++-V--+ +--V+ + | iommfd core || device| | vfio iommu | + +-+++ +---+ + +[Secure Context setup] +- iommufd BE: uses device fd and iommufd to setup secure context + (bind_iommufd, attach_ioas) +- vfio legacy BE: uses group fd and container fd to setup secure context + (set_container, set_iommu) + +[Device access] +- iommufd BE: device fd is opened through /dev/vfio/devices/vfioX +- vfio legacy BE: device fd is retrieved from group fd ioctl + +[DMA Mapping flow] +1. VFIOAddressSpace receives MemoryRegion add/del via MemoryListener +2. VFIO populates DMA map/unmap via the container BEs + *) iommufd BE: uses iommufd + *) vfio
Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
On 11/17/23 12:39, Duan, Zhenzhong wrote:
Hi Cédric,
-Original Message-
From: Cédric Le Goater
Sent: Friday, November 17, 2023 7:10 PM
Subject: Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
Hello,
+int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id,
hwaddr iova,
+ram_addr_t size, void *vaddr, bool readonly)
+{
+int ret, fd = be->fd;
+struct iommu_ioas_map map = {
+.size = sizeof(map),
+.flags = IOMMU_IOAS_MAP_READABLE |
+ IOMMU_IOAS_MAP_FIXED_IOVA,
+.ioas_id = ioas_id,
+.__reserved = 0,
+.user_va = (uintptr_t)vaddr,
+.iova = iova,
+.length = size,
+};
+
+if (!readonly) {
+map.flags |= IOMMU_IOAS_MAP_WRITEABLE;
+}
+
+ret = ioctl(fd, IOMMU_IOAS_MAP, );
+trace_iommufd_backend_map_dma(fd, ioas_id, iova, size,
+ vaddr, readonly, ret);
+if (ret) {
+ret = -errno;
+error_report("IOMMU_IOAS_MAP failed: %m");
+}
+return ret;
+}
When using a UEFI guest, QEMU reports errors when mapping regions
in the top PCI space :
iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38001000
size=0x3000 addr=0x7fce2c28b000 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
0x38001000, 0x3000, 0x7fce2c28b000) = -22 (Invalid argument)
iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38004000
size=0x4000 addr=0x7fce2c98 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
0x38004000, 0x4000, 0x7fce2c98) = -22 (Invalid argument)
This is because IOMMUFD reserved IOVAs areas are :
[ fee0 - feef ]
[ 80 - ] (39 bits address space)
which were allocated when the device was initially attached.
The topology is basic. Something is wrong.
Thanks for your report. This looks a hardware limit of
host IOMMU address width(39) < guest physical address width.
> A similar issue with a fix submitted below, ccing related people.
https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg02937.html
It looks the fix will not work for hotplug.
Or below qemu cmdline may help:
"-cpu host,host-phys-bits-limit=39"
Not that much. The IOMMU_IOAS_MAP failure becomes a "Bad address".
Thanks,
C.
Re: [PATCH v3 5/5] qom/object: Limit type names to alphanumerical and some few special characters
On 17/11/23 12:44, Thomas Huth wrote:
QOM names currently don't have any enforced naming rules. This
can be problematic, e.g. when they are used on the command line
for the "-device" option (where the comma is used to separate
properties). To avoid that such problematic type names come in
again, let's restrict the set of acceptable characters during the
type registration.
Ideally, we'd apply here the same rules as for QAPI, i.e. all type
names should begin with a letter, and contain only ASCII letters,
digits, hyphen, and underscore. However, we already have so many
pre-existing types like:
486-x86_64-cpu
cfi.pflash01
power5+_v2.1-spapr-cpu-core
virt-2.6-machine
pc-i440fx-3.0-machine
... so that we have to allow "." and "+" for now, too. While the
dot is used in a lot of places, the "+" can fortunately be limited
to two classes of legacy names ("power" and "Sun-UltraSparc" CPUs).
We also cannot enforce the rule that names must start with a letter
yet, since there are lot of types that start with a digit. Still,
at least limiting the first characters to the alphanumerical range
should be way better than nothing.
Signed-off-by: Thomas Huth
---
qom/object.c | 41 +
1 file changed, 41 insertions(+)
diff --git a/qom/object.c b/qom/object.c
index 95c0dc8285..654e1afaf2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -138,9 +138,50 @@ static TypeImpl *type_new(const TypeInfo *info)
return ti;
}
+static bool type_name_is_valid(const char *name)
+{
+const int slen = strlen(name);
+int plen;
+
+g_assert(slen > 1);
+
+/*
+ * Ideally, the name should start with a letter - however, we've got
+ * too many names starting with a digit already, so allow digits here,
+ * too (except '0' which is not used yet)
+ */
+if (!g_ascii_isalnum(name[0]) || name[0] == '0') {
+return false;
+}
+
+plen = strspn(name, "abcdefghijklmnopqrstuvwxyz"
+"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+"0123456789-_.");
+
+/* Allow some legacy names with '+' in it for compatibility reasons */
+if (name[plen] == '+') {
+if (plen == 6 && g_str_has_prefix(name, "power")) {
+/* Allow "power5+" and "power7+" CPU names*/
+return true;
+}
+if (plen >= 17 && g_str_has_prefix(name, "Sun-UltraSparc-I")) {
+/* Allow "Sun-UltraSparc-IV+" and "Sun-UltraSparc-IIIi+" */
+return true;
+}
+}
+
+return plen == slen;
+}
+
static TypeImpl *type_register_internal(const TypeInfo *info)
{
TypeImpl *ti;
+
+if (!type_name_is_valid(info->name)) {
+fprintf(stderr, "Registering '%s' with illegal type name\n",
info->name);
Shouldn't we use error_report() instead of fprintf()? Regardless,
Reviewed-by: Philippe Mathieu-Daudé
+abort();
+}
+
ti = type_new(info);
type_table_add(ti);
[PATCH v3 1/5] docs/system/arm: Fix for rename of type "xlnx.bbram-ctrl"
From: Markus Armbruster Fixes: b65b4b7ae3c8 (xlnx-bbram: hw/nvram: Use dot in device type name) Signed-off-by: Markus Armbruster [thuth: Use longhand syntax to avoid problems with the "." in the name] Reviewed-by: Peter Maydell Signed-off-by: Thomas Huth --- docs/system/arm/xlnx-versal-virt.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/xlnx-versal-virt.rst b/docs/system/arm/xlnx-versal-virt.rst index d2d1b26692..9a4b2ff55f 100644 --- a/docs/system/arm/xlnx-versal-virt.rst +++ b/docs/system/arm/xlnx-versal-virt.rst @@ -194,7 +194,7 @@ To use a different index value, N, from default of 0, add: .. code-block:: bash - -global xlnx,bbram-ctrl.drive-index=N + -global driver=xlnx.bbram-ctrl,property=drive-index,value=N eFUSE File Backend "" -- 2.42.0
[PATCH v3 3/5] memory: Remove "qemu:" prefix from the "qemu:ram-discard-manager" type name
Type names should not contain special characters like ":". Let's remove the whole prefix here since it does not really seem to be helpful to have such a prefix here. The type name is only used internally for an interface type, so the renaming should not affect the user interface or migration. Reviewed-by: David Hildenbrand Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- include/exec/memory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 831f7c996d..f172e82ac9 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -43,7 +43,7 @@ typedef struct IOMMUMemoryRegionClass IOMMUMemoryRegionClass; DECLARE_OBJ_CHECKERS(IOMMUMemoryRegion, IOMMUMemoryRegionClass, IOMMU_MEMORY_REGION, TYPE_IOMMU_MEMORY_REGION) -#define TYPE_RAM_DISCARD_MANAGER "qemu:ram-discard-manager" +#define TYPE_RAM_DISCARD_MANAGER "ram-discard-manager" typedef struct RamDiscardManagerClass RamDiscardManagerClass; typedef struct RamDiscardManager RamDiscardManager; DECLARE_OBJ_CHECKERS(RamDiscardManager, RamDiscardManagerClass, -- 2.42.0
[PATCH v3 2/5] hw: Replace anti-social QOM type names (again)
From: Markus Armbruster
QOM type names containing ',' result in awful UI. We got rid of them
in v6.0.0 (commit e178113ff64 hw: Replace anti-social QOM type names).
A few have crept back since:
xlnx,cframe-reg
xlnx,efuse
xlnx,pmc-efuse-cache
xlnx,versal-cfu-apb
xlnx,versal-cfu-fdro
xlnx,versal-cfu-sfr
xlnx,versal-crl
xlnx,versal-efuse
xlnx,zynqmp-efuse
These are all device types. They can't be plugged with -device /
device_add, except for "xlnx,efuse" (I'm not sure that one is
intentional).
They *can* be used with -device / device_add to request help.
Usability is poor, though: you have to double the comma, like this:
$ qemu-system-aarch64 -device xlnx,,pmc-efuse-cache,help
They can also be used with -global, where you must *not* double the
comma:
$ qemu-system-aarch64 -global xlnx,efuse.drive-index=2
Trap for the unwary.
"xlnx,efuse", "xlnx,versal-efuse", "xlnx,pmc-efuse-cache",
"xlnx-zynqmp-efuse" are from v6.2.0, "xlnx,versal-crl" is from v7.1.0,
and the remainder are new.
Rename them all to "xlnx-FOO", like commit e178113ff64 did.
Reported-by: Thomas Huth
Signed-off-by: Markus Armbruster
Reviewed-by: Thomas Huth
Reviewed-by: Francisco Iglesias
Signed-off-by: Thomas Huth
---
docs/system/arm/xlnx-versal-virt.rst | 2 +-
include/hw/misc/xlnx-versal-cframe-reg.h | 2 +-
include/hw/misc/xlnx-versal-cfu.h| 6 +++---
include/hw/misc/xlnx-versal-crl.h| 2 +-
include/hw/nvram/xlnx-efuse.h| 2 +-
include/hw/nvram/xlnx-versal-efuse.h | 4 ++--
include/hw/nvram/xlnx-zynqmp-efuse.h | 2 +-
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/docs/system/arm/xlnx-versal-virt.rst
b/docs/system/arm/xlnx-versal-virt.rst
index 9a4b2ff55f..0bafc76469 100644
--- a/docs/system/arm/xlnx-versal-virt.rst
+++ b/docs/system/arm/xlnx-versal-virt.rst
@@ -212,7 +212,7 @@ To use a different index value, N, from default of 1, add:
.. code-block:: bash
- -global xlnx,efuse.drive-index=N
+ -global xlnx-efuse.drive-index=N
.. warning::
In actual physical Versal, BBRAM and eFUSE contain sensitive data.
diff --git a/include/hw/misc/xlnx-versal-cframe-reg.h
b/include/hw/misc/xlnx-versal-cframe-reg.h
index a14fbd7fe4..f403b00e31 100644
--- a/include/hw/misc/xlnx-versal-cframe-reg.h
+++ b/include/hw/misc/xlnx-versal-cframe-reg.h
@@ -23,7 +23,7 @@
#include "hw/misc/xlnx-versal-cfu.h"
#include "qemu/fifo32.h"
-#define TYPE_XLNX_VERSAL_CFRAME_REG "xlnx,cframe-reg"
+#define TYPE_XLNX_VERSAL_CFRAME_REG "xlnx-cframe-reg"
OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFrameReg, XLNX_VERSAL_CFRAME_REG)
#define TYPE_XLNX_VERSAL_CFRAME_BCAST_REG "xlnx.cframe-bcast-reg"
diff --git a/include/hw/misc/xlnx-versal-cfu.h
b/include/hw/misc/xlnx-versal-cfu.h
index 86fb841053..8c581c0797 100644
--- a/include/hw/misc/xlnx-versal-cfu.h
+++ b/include/hw/misc/xlnx-versal-cfu.h
@@ -22,13 +22,13 @@
#include "hw/misc/xlnx-cfi-if.h"
#include "qemu/fifo32.h"
-#define TYPE_XLNX_VERSAL_CFU_APB "xlnx,versal-cfu-apb"
+#define TYPE_XLNX_VERSAL_CFU_APB "xlnx-versal-cfu-apb"
OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFUAPB, XLNX_VERSAL_CFU_APB)
-#define TYPE_XLNX_VERSAL_CFU_FDRO "xlnx,versal-cfu-fdro"
+#define TYPE_XLNX_VERSAL_CFU_FDRO "xlnx-versal-cfu-fdro"
OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFUFDRO, XLNX_VERSAL_CFU_FDRO)
-#define TYPE_XLNX_VERSAL_CFU_SFR "xlnx,versal-cfu-sfr"
+#define TYPE_XLNX_VERSAL_CFU_SFR "xlnx-versal-cfu-sfr"
OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFUSFR, XLNX_VERSAL_CFU_SFR)
REG32(CFU_ISR, 0x0)
diff --git a/include/hw/misc/xlnx-versal-crl.h
b/include/hw/misc/xlnx-versal-crl.h
index 2857f4169a..dfb8dff197 100644
--- a/include/hw/misc/xlnx-versal-crl.h
+++ b/include/hw/misc/xlnx-versal-crl.h
@@ -13,7 +13,7 @@
#include "hw/register.h"
#include "target/arm/cpu.h"
-#define TYPE_XLNX_VERSAL_CRL "xlnx,versal-crl"
+#define TYPE_XLNX_VERSAL_CRL "xlnx-versal-crl"
OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCRL, XLNX_VERSAL_CRL)
REG32(ERR_CTRL, 0x0)
diff --git a/include/hw/nvram/xlnx-efuse.h b/include/hw/nvram/xlnx-efuse.h
index 58414e468b..cff7924106 100644
--- a/include/hw/nvram/xlnx-efuse.h
+++ b/include/hw/nvram/xlnx-efuse.h
@@ -30,7 +30,7 @@
#include "sysemu/block-backend.h"
#include "hw/qdev-core.h"
-#define TYPE_XLNX_EFUSE "xlnx,efuse"
+#define TYPE_XLNX_EFUSE "xlnx-efuse"
OBJECT_DECLARE_SIMPLE_TYPE(XlnxEFuse, XLNX_EFUSE);
struct XlnxEFuse {
diff --git a/include/hw/nvram/xlnx-versal-efuse.h
b/include/hw/nvram/xlnx-versal-efuse.h
index a873dc5cb0..86e2261b9a 100644
--- a/include/hw/nvram/xlnx-versal-efuse.h
+++ b/include/hw/nvram/xlnx-versal-efuse.h
@@ -29,8 +29,8 @@
#define XLNX_VERSAL_EFUSE_CTRL_R_MAX ((0x100 / 4) + 1)
-#define TYPE_XLNX_VERSAL_EFUSE_CTRL "xlnx,versal-efuse"
-#define TYPE_XLNX_VERSAL_EFUSE_CACHE "xlnx,pmc-efuse-cache"
+#define TYPE_XLNX_VERSAL_EFUSE_CTRL "xlnx-versal-efuse"
+#define TYPE_XLNX_VERSAL_EFUSE_CACHE "xlnx-pmc-efuse-cache"
[PATCH v3 4/5] tests/unit/test-io-task: Rename "qemu:dummy" to avoid colon in the name
Type names should not contain special characters like ":" (so that they are easier to use with QAPI and other parts). We are going to forbid such names in an upcoming patch. Thus let's replace the ":" here with a "-". Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/unit/test-io-task.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test-io-task.c b/tests/unit/test-io-task.c index 953a50ae66..115dba8970 100644 --- a/tests/unit/test-io-task.c +++ b/tests/unit/test-io-task.c @@ -25,7 +25,7 @@ #include "qapi/error.h" #include "qemu/module.h" -#define TYPE_DUMMY "qemu:dummy" +#define TYPE_DUMMY "qemu-dummy" typedef struct DummyObject DummyObject; typedef struct DummyObjectClass DummyObjectClass; -- 2.42.0
[PATCH v3 5/5] qom/object: Limit type names to alphanumerical and some few special characters
QOM names currently don't have any enforced naming rules. This
can be problematic, e.g. when they are used on the command line
for the "-device" option (where the comma is used to separate
properties). To avoid that such problematic type names come in
again, let's restrict the set of acceptable characters during the
type registration.
Ideally, we'd apply here the same rules as for QAPI, i.e. all type
names should begin with a letter, and contain only ASCII letters,
digits, hyphen, and underscore. However, we already have so many
pre-existing types like:
486-x86_64-cpu
cfi.pflash01
power5+_v2.1-spapr-cpu-core
virt-2.6-machine
pc-i440fx-3.0-machine
... so that we have to allow "." and "+" for now, too. While the
dot is used in a lot of places, the "+" can fortunately be limited
to two classes of legacy names ("power" and "Sun-UltraSparc" CPUs).
We also cannot enforce the rule that names must start with a letter
yet, since there are lot of types that start with a digit. Still,
at least limiting the first characters to the alphanumerical range
should be way better than nothing.
Signed-off-by: Thomas Huth
---
qom/object.c | 41 +
1 file changed, 41 insertions(+)
diff --git a/qom/object.c b/qom/object.c
index 95c0dc8285..654e1afaf2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -138,9 +138,50 @@ static TypeImpl *type_new(const TypeInfo *info)
return ti;
}
+static bool type_name_is_valid(const char *name)
+{
+const int slen = strlen(name);
+int plen;
+
+g_assert(slen > 1);
+
+/*
+ * Ideally, the name should start with a letter - however, we've got
+ * too many names starting with a digit already, so allow digits here,
+ * too (except '0' which is not used yet)
+ */
+if (!g_ascii_isalnum(name[0]) || name[0] == '0') {
+return false;
+}
+
+plen = strspn(name, "abcdefghijklmnopqrstuvwxyz"
+"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+"0123456789-_.");
+
+/* Allow some legacy names with '+' in it for compatibility reasons */
+if (name[plen] == '+') {
+if (plen == 6 && g_str_has_prefix(name, "power")) {
+/* Allow "power5+" and "power7+" CPU names*/
+return true;
+}
+if (plen >= 17 && g_str_has_prefix(name, "Sun-UltraSparc-I")) {
+/* Allow "Sun-UltraSparc-IV+" and "Sun-UltraSparc-IIIi+" */
+return true;
+}
+}
+
+return plen == slen;
+}
+
static TypeImpl *type_register_internal(const TypeInfo *info)
{
TypeImpl *ti;
+
+if (!type_name_is_valid(info->name)) {
+fprintf(stderr, "Registering '%s' with illegal type name\n",
info->name);
+abort();
+}
+
ti = type_new(info);
type_table_add(ti);
--
2.42.0
[PATCH v3 0/5] Limit type names to alphanumerical and some few special characters
QOM names currently don't have any enforced naming rules. This can be problematic, e.g. when they are used on the command line for the "-device" option (where the comma is used to separate properties). To avoid that such problematic type names come in again, let's restrict the set of acceptable characters during the type registration. First four patches are clean-ups related to comma and colons in type names, and the final patch introduces the check for valid names. v3: - Added Reviewed-bys to the first four patches - Changed last patch to use strspn() as suggested by Daniel v2: - Include Markus' patches in the series - Add patches to clean up colons in type names - Add the check to type_register_internal() instead of type_new() so that we can disallow colons, too - Only allow '+' in legacy names Markus Armbruster (2): docs/system/arm: Fix for rename of type "xlnx.bbram-ctrl" hw: Replace anti-social QOM type names (again) Thomas Huth (3): memory: Remove "qemu:" prefix from the "qemu:ram-discard-manager" type name tests/unit/test-io-task: Rename "qemu:dummy" to avoid colon in the name qom/object: Limit type names to alphanumerical and some few special characters docs/system/arm/xlnx-versal-virt.rst | 4 +-- include/exec/memory.h| 2 +- include/hw/misc/xlnx-versal-cframe-reg.h | 2 +- include/hw/misc/xlnx-versal-cfu.h| 6 ++-- include/hw/misc/xlnx-versal-crl.h| 2 +- include/hw/nvram/xlnx-efuse.h| 2 +- include/hw/nvram/xlnx-versal-efuse.h | 4 +-- include/hw/nvram/xlnx-zynqmp-efuse.h | 2 +- qom/object.c | 41 tests/unit/test-io-task.c| 2 +- 10 files changed, 54 insertions(+), 13 deletions(-) -- 2.42.0
RE: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
Hi Cédric,
>-Original Message-
>From: Cédric Le Goater
>Sent: Friday, November 17, 2023 7:10 PM
>Subject: Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
>
>Hello,
>
>> +int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id,
>hwaddr iova,
>> +ram_addr_t size, void *vaddr, bool readonly)
>> +{
>> +int ret, fd = be->fd;
>> +struct iommu_ioas_map map = {
>> +.size = sizeof(map),
>> +.flags = IOMMU_IOAS_MAP_READABLE |
>> + IOMMU_IOAS_MAP_FIXED_IOVA,
>> +.ioas_id = ioas_id,
>> +.__reserved = 0,
>> +.user_va = (uintptr_t)vaddr,
>> +.iova = iova,
>> +.length = size,
>> +};
>> +
>> +if (!readonly) {
>> +map.flags |= IOMMU_IOAS_MAP_WRITEABLE;
>> +}
>> +
>> +ret = ioctl(fd, IOMMU_IOAS_MAP, );
>> +trace_iommufd_backend_map_dma(fd, ioas_id, iova, size,
>> + vaddr, readonly, ret);
>> +if (ret) {
>> +ret = -errno;
>> +error_report("IOMMU_IOAS_MAP failed: %m");
>> +}
>> +return ret;
>> +}
>
>When using a UEFI guest, QEMU reports errors when mapping regions
>in the top PCI space :
>
> iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38001000
>size=0x3000 addr=0x7fce2c28b000 readonly=0 (-1)
> qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
> qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
>0x38001000, 0x3000, 0x7fce2c28b000) = -22 (Invalid argument)
>
> iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38004000
>size=0x4000 addr=0x7fce2c98 readonly=0 (-1)
> qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
> qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150,
>0x38004000, 0x4000, 0x7fce2c98) = -22 (Invalid argument)
>
>This is because IOMMUFD reserved IOVAs areas are :
>
> [ fee0 - feef ]
> [ 80 - ] (39 bits address space)
>
>which were allocated when the device was initially attached.
>The topology is basic. Something is wrong.
Thanks for your report. This looks a hardware limit of
host IOMMU address width(39) < guest physical address width.
A similar issue with a fix submitted below, ccing related people.
https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg02937.html
It looks the fix will not work for hotplug.
Or below qemu cmdline may help:
"-cpu host,host-phys-bits-limit=39"
Thanks
Zhenzhong
Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
Hello,
+int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas_id, hwaddr iova,
+ram_addr_t size, void *vaddr, bool readonly)
+{
+int ret, fd = be->fd;
+struct iommu_ioas_map map = {
+.size = sizeof(map),
+.flags = IOMMU_IOAS_MAP_READABLE |
+ IOMMU_IOAS_MAP_FIXED_IOVA,
+.ioas_id = ioas_id,
+.__reserved = 0,
+.user_va = (uintptr_t)vaddr,
+.iova = iova,
+.length = size,
+};
+
+if (!readonly) {
+map.flags |= IOMMU_IOAS_MAP_WRITEABLE;
+}
+
+ret = ioctl(fd, IOMMU_IOAS_MAP, );
+trace_iommufd_backend_map_dma(fd, ioas_id, iova, size,
+ vaddr, readonly, ret);
+if (ret) {
+ret = -errno;
+error_report("IOMMU_IOAS_MAP failed: %m");
+}
+return ret;
+}
When using a UEFI guest, QEMU reports errors when mapping regions
in the top PCI space :
iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38001000 size=0x3000
addr=0x7fce2c28b000 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150, 0x38001000,
0x3000, 0x7fce2c28b000) = -22 (Invalid argument)
iommufd_backend_map_dma iommufd=10 ioas=2 iova=0x38004000 size=0x4000
addr=0x7fce2c98 readonly=0 (-1)
qemu-system-x86_64: IOMMU_IOAS_MAP failed: Invalid argument
qemu-system-x86_64: vfio_container_dma_map(0x55a21b03a150, 0x38004000,
0x4000, 0x7fce2c98) = -22 (Invalid argument)
This is because IOMMUFD reserved IOVAs areas are :
[ fee0 - feef ]
[ 80 - ] (39 bits address space)
which were allocated when the device was initially attached.
The topology is basic. Something is wrong.
Thanks,
C.
[PATCH v2 0/2] HPPA64-PATCHES-for-8.2
From: Helge Deller Two patches which I'd like to get included for 8.2. The SHRPD patch fixes a real translation bug which then allows to boot the 64-bit Linux kernels of the Debian-11 and Debian-12 installation CDs. The second patch adds the instruction byte sequence to the assembly log. This is not an actual bug fix, but it's important since it helps a lot when trying to fix qemu translation bugs on hppa. v2: - corrected "upper" and "lower" in commit SHRPD message Helge Deller (2): target/hppa: Fix 64-bit SHRPD instruction disas/hppa: Show hexcode of instruction along with disassembly disas/hppa.c| 3 +++ target/hppa/translate.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) -- 2.41.0
[PATCH v2 1/2] target/hppa: Fix 64-bit SHRPD instruction
From: Helge Deller
When shifting the two joined 64-bit registers right, shift the upper
64-bit register to the left and the lower 64-bit register to the right
before merging them with OR.
Signed-off-by: Helge Deller
---
target/hppa/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 4a4830c3e3..3ef39b1bd7 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3438,9 +3438,9 @@ static bool trans_shrp_sar(DisasContext *ctx,
arg_shrp_sar *a)
TCGv_i64 n = tcg_temp_new_i64();
tcg_gen_xori_i64(n, cpu_sar, 63);
-tcg_gen_shl_i64(t, src2, n);
+tcg_gen_shl_i64(t, src1, n);
tcg_gen_shli_i64(t, t, 1);
-tcg_gen_shr_i64(dest, src1, cpu_sar);
+tcg_gen_shr_i64(dest, src2, cpu_sar);
tcg_gen_or_i64(dest, dest, t);
} else {
TCGv_i64 t = tcg_temp_new_i64();
--
2.41.0
[PATCH v2 2/2] disas/hppa: Show hexcode of instruction along with disassembly
From: Helge Deller
On hppa many instructions can be expressed by different bytecodes.
To be able to debug qemu translation bugs it's therefore necessary to see the
currently executed byte codes without the need to lookup the sequence without
the full executable.
With this patch the instruction byte code is shown beside the disassembly.
Signed-off-by: Helge Deller
---
disas/hppa.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/disas/hppa.c b/disas/hppa.c
index dcf9a47f34..38fc05acc4 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1979,6 +1979,9 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
if (opcode->arch == pa20w)
continue;
#endif
+ (*info->fprintf_func) (info->stream, " %02x %02x %02x %02x ",
+(insn >> 24) & 0xff, (insn >> 16) & 0xff,
+(insn >> 8) & 0xff, insn & 0xff);
(*info->fprintf_func) (info->stream, "%s", opcode->name);
if (!strchr ("cfCY?-+nHNZFIuv{", opcode->args[0]))
--
2.41.0
[PATCH 0/2] HPPA64-PATCHES-for-8.2
From: Helge Deller Two HPPA64 patches which I'd like to get included for 8.2. The SHRPD patch fixes a real translation bug which then allows to boot the 64-bit Linux kernels of the Debian-11 and Debian-12 installation CDs. The second patch adds the instruction byte sequence to the assembly log. This is not an actual bug fix, but it's important since it helps a lot when trying to fix qemu translation bugs on hppa. Helge Helge Deller (2): target/hppa: Fix 64-bit SHRPD instruction disas/hppa: Show hexcode of instruction along with disassembly disas/hppa.c| 3 +++ target/hppa/translate.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) -- 2.41.0
[PATCH 2/2] disas/hppa: Show hexcode of instruction along with disassembly
From: Helge Deller
On hppa many instructions can be expressed by different bytecodes.
To be able to debug qemu translation bugs it's therefore necessary to see the
currently executed byte codes without the need to lookup the sequence without
the full executable.
With this patch the instruction byte code is shown beside the disassembly.
Signed-off-by: Helge Deller
---
disas/hppa.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/disas/hppa.c b/disas/hppa.c
index dcf9a47f34..38fc05acc4 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1979,6 +1979,9 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
if (opcode->arch == pa20w)
continue;
#endif
+ (*info->fprintf_func) (info->stream, " %02x %02x %02x %02x ",
+(insn >> 24) & 0xff, (insn >> 16) & 0xff,
+(insn >> 8) & 0xff, insn & 0xff);
(*info->fprintf_func) (info->stream, "%s", opcode->name);
if (!strchr ("cfCY?-+nHNZFIuv{", opcode->args[0]))
--
2.41.0
[PATCH 1/2] target/hppa: Fix 64-bit SHRPD instruction
From: Helge Deller
When shifting the two joined 64-bit registers right, shift the lower
64-bit register to the left and the higher 64-bit register to the right
before merging them with OR.
Signed-off-by: Helge Deller
---
target/hppa/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 4a4830c3e3..3ef39b1bd7 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3438,9 +3438,9 @@ static bool trans_shrp_sar(DisasContext *ctx,
arg_shrp_sar *a)
TCGv_i64 n = tcg_temp_new_i64();
tcg_gen_xori_i64(n, cpu_sar, 63);
-tcg_gen_shl_i64(t, src2, n);
+tcg_gen_shl_i64(t, src1, n);
tcg_gen_shli_i64(t, t, 1);
-tcg_gen_shr_i64(dest, src1, cpu_sar);
+tcg_gen_shr_i64(dest, src2, cpu_sar);
tcg_gen_or_i64(dest, dest, t);
} else {
TCGv_i64 t = tcg_temp_new_i64();
--
2.41.0
RE: [PATCH v5 11/11] tests/qtest: Adding PCS Module test to GMAC Qtest
-Original Message-
From: Nabih Estefan
Sent: Saturday, October 28, 2023 1:56 AM
To: peter.mayd...@linaro.org
Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; CS20 KFTing
; wuhao...@google.com; jasonw...@redhat.com; IS20 Avi
Fishman ; nabiheste...@google.com; CS20 KWLiu
; IS20 Tomer Maimon ; IN20 Hila
Miranda-Kuzi
Subject: [PATCH v5 11/11] tests/qtest: Adding PCS Module test to GMAC Qtest
From: Nabih Estefan Diaz
- Add PCS Register check to npcm_gmac-test
Change-Id: I4e9cc0e1056f35467252c7bf6bd5b44fef61b6e8
Signed-off-by: Nabih Estefan
---
tests/qtest/npcm_gmac-test.c | 134 ++-
1 file changed, 133 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/npcm_gmac-test.c b/tests/qtest/npcm_gmac-test.c index
130a1599a8..0958b13814 100644
--- a/tests/qtest/npcm_gmac-test.c
+++ b/tests/qtest/npcm_gmac-test.c
@@ -20,6 +20,10 @@
/* Name of the GMAC Device */
#define TYPE_NPCM_GMAC "npcm-gmac"
+/* Address of the PCS Module */
+#define PCS_BASE_ADDRESS 0xf078
+#define NPCM_PCS_IND_AC_BA 0x1fe
+
typedef struct GMACModule {
int irq;
uint64_t base_addr;
@@ -111,6 +115,62 @@ typedef enum NPCMRegister {
NPCM_GMAC_PTP_STNSUR = 0x714,
NPCM_GMAC_PTP_TAR = 0x718,
NPCM_GMAC_PTP_TTSR = 0x71c,
+
+/* PCS Registers */
+NPCM_PCS_SR_CTL_ID1 = 0x3c0008,
+NPCM_PCS_SR_CTL_ID2 = 0x3c000a,
+NPCM_PCS_SR_CTL_STS = 0x3c0010,
+
+NPCM_PCS_SR_MII_CTRL = 0x3e,
+NPCM_PCS_SR_MII_STS = 0x3e0002,
+NPCM_PCS_SR_MII_DEV_ID1 = 0x3e0004,
+NPCM_PCS_SR_MII_DEV_ID2 = 0x3e0006,
+NPCM_PCS_SR_MII_AN_ADV = 0x3e0008,
+NPCM_PCS_SR_MII_LP_BABL = 0x3e000a,
+NPCM_PCS_SR_MII_AN_EXPN = 0x3e000c,
+NPCM_PCS_SR_MII_EXT_STS = 0x3e001e,
+
+NPCM_PCS_SR_TIM_SYNC_ABL = 0x3e0e10,
+NPCM_PCS_SR_TIM_SYNC_TX_MAX_DLY_LWR = 0x3e0e12,
+NPCM_PCS_SR_TIM_SYNC_TX_MAX_DLY_UPR = 0x3e0e14,
+NPCM_PCS_SR_TIM_SYNC_TX_MIN_DLY_LWR = 0x3e0e16,
+NPCM_PCS_SR_TIM_SYNC_TX_MIN_DLY_UPR = 0x3e0e18,
+NPCM_PCS_SR_TIM_SYNC_RX_MAX_DLY_LWR = 0x3e0e1a,
+NPCM_PCS_SR_TIM_SYNC_RX_MAX_DLY_UPR = 0x3e0e1c,
+NPCM_PCS_SR_TIM_SYNC_RX_MIN_DLY_LWR = 0x3e0e1e,
+NPCM_PCS_SR_TIM_SYNC_RX_MIN_DLY_UPR = 0x3e0e20,
+
+NPCM_PCS_VR_MII_MMD_DIG_CTRL1 = 0x3f,
+NPCM_PCS_VR_MII_AN_CTRL = 0x3f0002,
+NPCM_PCS_VR_MII_AN_INTR_STS = 0x3f0004,
+NPCM_PCS_VR_MII_TC = 0x3f0006,
+NPCM_PCS_VR_MII_DBG_CTRL = 0x3f000a,
+NPCM_PCS_VR_MII_EEE_MCTRL0 = 0x3f000c,
+NPCM_PCS_VR_MII_EEE_TXTIMER = 0x3f0010,
+NPCM_PCS_VR_MII_EEE_RXTIMER = 0x3f0012,
+NPCM_PCS_VR_MII_LINK_TIMER_CTRL = 0x3f0014,
+NPCM_PCS_VR_MII_EEE_MCTRL1 = 0x3f0016,
+NPCM_PCS_VR_MII_DIG_STS = 0x3f0020,
+NPCM_PCS_VR_MII_ICG_ERRCNT1 = 0x3f0022,
+NPCM_PCS_VR_MII_MISC_STS = 0x3f0030,
+NPCM_PCS_VR_MII_RX_LSTS = 0x3f0040,
+NPCM_PCS_VR_MII_MP_TX_BSTCTRL0 = 0x3f0070,
+NPCM_PCS_VR_MII_MP_TX_LVLCTRL0 = 0x3f0074,
+NPCM_PCS_VR_MII_MP_TX_GENCTRL0 = 0x3f007a,
+NPCM_PCS_VR_MII_MP_TX_GENCTRL1 = 0x3f007c,
+NPCM_PCS_VR_MII_MP_TX_STS = 0x3f0090,
+NPCM_PCS_VR_MII_MP_RX_GENCTRL0 = 0x3f00b0,
+NPCM_PCS_VR_MII_MP_RX_GENCTRL1 = 0x3f00b2,
+NPCM_PCS_VR_MII_MP_RX_LOS_CTRL0 = 0x3f00ba,
+NPCM_PCS_VR_MII_MP_MPLL_CTRL0 = 0x3f00f0,
+NPCM_PCS_VR_MII_MP_MPLL_CTRL1 = 0x3f00f2,
+NPCM_PCS_VR_MII_MP_MPLL_STS = 0x3f0110,
+NPCM_PCS_VR_MII_MP_MISC_CTRL2 = 0x3f0126,
+NPCM_PCS_VR_MII_MP_LVL_CTRL = 0x3f0130,
+NPCM_PCS_VR_MII_MP_MISC_CTRL0 = 0x3f0132,
+NPCM_PCS_VR_MII_MP_MISC_CTRL1 = 0x3f0134,
+NPCM_PCS_VR_MII_DIG_CTRL2 = 0x3f01c2,
+NPCM_PCS_VR_MII_DIG_ERRCNT_SEL = 0x3f01c4,
} NPCMRegister;
static uint32_t gmac_read(QTestState *qts, const GMACModule *mod, @@ -119,6
+179,15 @@ static uint32_t gmac_read(QTestState *qts, const GMACModule *mod,
return qtest_readl(qts, mod->base_addr + regno); }
+static uint16_t pcs_read(QTestState *qts, const GMACModule *mod,
+ NPCMRegister regno) {
+uint32_t write_value = (regno & 0x3ffe00) >> 9;
+qtest_writel(qts, PCS_BASE_ADDRESS + NPCM_PCS_IND_AC_BA, write_value);
+uint32_t read_offset = regno & 0x1ff;
+return qtest_readl(qts, PCS_BASE_ADDRESS + read_offset); }
+
/* Check that GMAC registers are reset to default value */ static void
test_init(gconstpointer test_data) { @@ -129,7 +198,12 @@ static void
test_init(gconstpointer test_data) #define CHECK_REG32(regno, value) \
do { \
g_assert_cmphex(gmac_read(qts, mod, (regno)), ==, (value)); \
-} while (0)
+} while (0) ;
+
+#define CHECK_REG_PCS(regno, value) \
+do { \
+g_assert_cmphex(pcs_read(qts, mod, (regno)), ==, (value)); \
+} while (0) ;
CHECK_REG32(NPCM_DMA_BUS_MODE, 0x00020100);
CHECK_REG32(NPCM_DMA_XMT_POLL_DEMAND, 0); @@ -180,6 +254,64 @@ static void
test_init(gconstpointer test_data)
CHECK_REG32(NPCM_GMAC_PTP_TAR, 0);
CHECK_REG32(NPCM_GMAC_PTP_TTSR, 0);
+/* TODO Add registers PCS */
+if (mod->base_addr == 0xf0802000) {
Re: [RFC 0/2] vhost-user-test: Add negotiated features check
On Thu, Nov 16, 2023 at 09:01:28AM +0800, Yong Huang wrote: > ping Sit tight pls it's only been a couple of days. But if you want to, address comments by Markus pls.
Re: [PATCH for-8.2] Revert "ui/console: allow to override the default VC"
Hi
On Thu, Nov 16, 2023 at 10:25 PM Peter Maydell wrote:
>
> This reverts commit 1bec1cc0da497e55c16e2a7b50f94cdb2a02197f. This
> commit changed the behaviour of the "-display none" option, so that
> it now creates a QEMU monitor on the terminal. "-display none"
> should not be tangled up with whether we create a monitor or a serial
> terminal; it should purely and only disable the graphical window.
> Changing its behaviour like this breaks command lines which, for
> example, use semihosting for their output and don't want a graphical
> window, as they now get a monitor they never asked for.
>
> It also breaks the command line we document for Xen in
> docs/system/i386/xen.html:
>
> $ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split \
> -display none -chardev stdio,mux=on,id=char0,signal=off -mon char0 \
> -device xen-console,chardev=char0 -drive file=${GUEST_IMAGE},if=xen
>
> qemu-system-x86_64: cannot use stdio by multiple character devices
> qemu-system-x86_64: could not connect serial device to character backend
> 'stdio'
>
> Revert the commit to restore the previous handling of "-display
> none".
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1974
> Signed-off-by: Peter Maydell
By simply reverting, we break qemu with --disable-pixman:
qemu-system-x86_64: 'vc' is not a valid char driver name
The change of behaviour was a consequence of Paolo suggestion from v2:
https://patchew.org/QEMU/20230918135206.2739222-1-marcandre.lur...@redhat.com/20230918135206.2739222-6-marcandre.lur...@redhat.com/#4b890258-3426-0e0f-dd65-6114b9bee...@redhat.com
Let's get back to the current behaviour and not add muxed console in
any case (disable pixman or not). I'll try to make a patch asap.
> ---
> include/ui/console.h | 2 --
> system/vl.c | 27 ++-
> ui/console.c | 17 -
> 3 files changed, 10 insertions(+), 36 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index a4a49ffc640..acb61a7f152 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -462,14 +462,12 @@ struct QemuDisplay {
> DisplayType type;
> void (*early_init)(DisplayOptions *opts);
> void (*init)(DisplayState *ds, DisplayOptions *opts);
> -const char *vc;
> };
>
> void qemu_display_register(QemuDisplay *ui);
> bool qemu_display_find_default(DisplayOptions *opts);
> void qemu_display_early_init(DisplayOptions *opts);
> void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
> -const char *qemu_display_get_vc(DisplayOptions *opts);
> void qemu_display_help(void);
>
> /* vnc.c */
> diff --git a/system/vl.c b/system/vl.c
> index 5af7ced2a16..3d64a90f253 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1372,7 +1372,6 @@ static void qemu_setup_display(void)
> static void qemu_create_default_devices(void)
> {
> MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
> -const char *vc = qemu_display_get_vc();
>
> if (is_daemonized()) {
> /* According to documentation and historically, -nographic redirects
> @@ -1391,30 +1390,24 @@ static void qemu_create_default_devices(void)
> }
> }
>
> -if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) {
> -if (default_parallel) {
> +if (nographic) {
> +if (default_parallel)
> add_device_config(DEV_PARALLEL, "null");
> -}
> if (default_serial && default_monitor) {
> add_device_config(DEV_SERIAL, "mon:stdio");
> } else {
> -if (default_serial) {
> +if (default_serial)
> add_device_config(DEV_SERIAL, "stdio");
> -}
> -if (default_monitor) {
> +if (default_monitor)
> monitor_parse("stdio", "readline", false);
> -}
> }
> } else {
> -if (default_serial) {
> -add_device_config(DEV_SERIAL, vc ?: "null");
> -}
> -if (default_parallel) {
> -add_device_config(DEV_PARALLEL, vc ?: "null");
> -}
> -if (default_monitor && vc) {
> -monitor_parse(vc, "readline", false);
> -}
> +if (default_serial)
> +add_device_config(DEV_SERIAL, "vc:80Cx24C");
> +if (default_parallel)
> +add_device_config(DEV_PARALLEL, "vc:80Cx24C");
> +if (default_monitor)
> +monitor_parse("vc:80Cx24C", "readline", false);
> }
>
> if (default_net) {
> diff --git a/ui/console.c b/ui/console.c
> index 8e688d35695..676d0cbaba2 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1677,23 +1677,6 @@ void qemu_display_init(DisplayState *ds,
> DisplayOptions *opts)
> dpys[opts->type]->init(ds, opts);
> }
>
> -const char *qemu_display_get_vc(DisplayOptions *opts)
> -{
> -assert(opts->type < DISPLAY_TYPE__MAX);
> -if (opts->type == DISPLAY_TYPE_NONE) {
> -
Re: [PATCH] riscv: Fix SiFive E CLINT clock frequency
On 11/17/23 05:28, Román Cárdenas wrote: If you check the manual of SiFive E310 (https://cdn.sparkfun.com/assets/7/f/0/2/7/fe310-g002-manual-v19p05.pdf), you can see in Figure 1 that the CLINT is connected to the real time clock, which also feeds the AON peripheral (they share the same clock). In page 43, the docs also say that the timer registers of the CLINT count ticks from the rtcclk. I am currently playing with bare metal applications both in QEMU and a physical SiFive E310 board and I confirm that the CLINT clock in the physical board runs at 32.768 kHz. In QEMU, the same app produces a completely different outcome, as sometimes a new CLINT interrupt is triggered before finishing other tasks. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1978 Signed-off-by: Román Cárdenas --- As I said in an earlier version, it would be nice if someone from Sifive could give an ACK/NACK for this change. For now: Reviewed-by: Daniel Henrique Barboza hw/riscv/sifive_e.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 0d37adc542..87d9602383 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -225,7 +225,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp) RISCV_ACLINT_SWI_SIZE, RISCV_ACLINT_DEFAULT_MTIMER_SIZE, 0, ms->smp.cpus, RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME, -RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, false); +SIFIVE_E_LFCLK_DEFAULT_FREQ, false); sifive_e_prci_create(memmap[SIFIVE_E_DEV_PRCI].base); /* AON */
[PULL 0/7] Error reporting patches for 2023-11-17
The following changes since commit 34a5cb6d8434303c170230644b2a7c1d5781d197: Merge tag 'pull-tcg-20231114' of https://gitlab.com/rth7680/qemu into staging (2023-11-15 08:05:25 -0500) are available in the Git repository at: https://repo.or.cz/qemu/armbru.git tags/pull-error-2023-11-17 for you to fetch changes up to 298d8b122056052951bda487392d8aabbfd0f3e5: target/i386/cpu: Improve error message for property "vendor" (2023-11-17 10:07:52 +0100) Error reporting patches for 2023-11-17 Markus Armbruster (7): spapr/pci: Correct "does not support hotplugging error messages hmp: Improve sync-profile error message qga: Improve guest-exec-status error message ui/qmp-cmds: Improve two error messages net: Fix a misleading error message balloon: Fix a misleading error message target/i386/cpu: Improve error message for property "vendor" hw/ppc/spapr_pci.c | 4 ++-- monitor/hmp-cmds.c | 4 ++-- net/net.c | 6 +++--- qga/commands.c | 2 +- system/balloon.c | 10 +- target/i386/cpu.c | 3 ++- ui/ui-qmp-cmds.c | 5 +++-- 7 files changed, 18 insertions(+), 16 deletions(-) -- 2.41.0
[PULL 1/7] spapr/pci: Correct "does not support hotplugging error messages
When dynamic-reconfiguration is off, hot plug / unplug can fail with
"Bus 'spapr-pci-host-bridge' does not support hotplugging".
spapr-pci-host-bridge is a device, not a bus. Report the name of the
bus it provides instead: 'pci.0'.
Signed-off-by: Markus Armbruster
Message-ID: <2023103059.3407803-2-arm...@redhat.com>
Reviewed-by: Daniel Henrique Barboza
---
hw/ppc/spapr_pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a27024e45a..6760823e13 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1555,7 +1555,7 @@ static void spapr_pci_pre_plug(HotplugHandler
*plug_handler,
*/
if (plugged_dev->hotplugged) {
error_setg(errp, QERR_BUS_NO_HOTPLUG,
- object_get_typename(OBJECT(phb)));
+ phb->parent_obj.bus->qbus.name);
return;
}
}
@@ -1676,7 +1676,7 @@ static void spapr_pci_unplug_request(HotplugHandler
*plug_handler,
if (!phb->dr_enabled) {
error_setg(errp, QERR_BUS_NO_HOTPLUG,
- object_get_typename(OBJECT(phb)));
+ phb->parent_obj.bus->qbus.name);
return;
}
--
2.41.0
[PULL 7/7] target/i386/cpu: Improve error message for property "vendor"
Improve
$ qemu-system-x86_64 -device max-x86_64-cpu,vendor=me
qemu-system-x86_64: -device max-x86_64-cpu,vendor=me: Property '.vendor'
doesn't take value 'me'
to
qemu-system-x86_64: -device max-x86_64-cpu,vendor=0123456789abc: value of
property 'vendor' must consist of exactly 12 characters
Signed-off-by: Markus Armbruster
Message-ID: <2023103059.3407803-8-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé
[Typo corrected]
---
target/i386/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 358d9c0a65..cd16cb893d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5192,7 +5192,8 @@ static void x86_cpuid_set_vendor(Object *obj, const char
*value,
int i;
if (strlen(value) != CPUID_VENDOR_SZ) {
-error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value);
+error_setg(errp, "value of property 'vendor' must consist of"
+ " exactly " stringify(CPUID_VENDOR_SZ) " characters");
return;
}
--
2.41.0
[PULL 2/7] hmp: Improve sync-profile error message
Improve
(qemu) sync-profile of
Error: Invalid parameter 'of'
to
Error: invalid parameter 'of', expecting 'on', 'off', or 'reset'
Signed-off-by: Markus Armbruster
Message-ID: <2023103059.3407803-3-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Dr. David Alan Gilbert
---
monitor/hmp-cmds.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 6c559b48c8..871898ac46 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -24,7 +24,6 @@
#include "qapi/qapi-commands-control.h"
#include "qapi/qapi-commands-misc.h"
#include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qerror.h"
#include "qemu/cutils.h"
#include "hw/intc/intc.h"
#include "qemu/log.h"
@@ -138,7 +137,8 @@ void hmp_sync_profile(Monitor *mon, const QDict *qdict)
} else {
Error *err = NULL;
-error_setg(, QERR_INVALID_PARAMETER, op);
+error_setg(, "invalid parameter '%s',"
+ " expecting 'on', 'off', or 'reset'", op);
hmp_handle_error(mon, err);
}
}
--
2.41.0
[PULL 3/7] qga: Improve guest-exec-status error message
When the PID passed to guest-exec-status does not exist, we report
"Invalid parameter 'pid'"
Improve this to
"PID 1234 does not exist"
Signed-off-by: Markus Armbruster
Message-ID: <2023103059.3407803-4-arm...@redhat.com>
Reviewed-by: Konstantin Kostiuk
Reviewed-by: Philippe Mathieu-Daudé
---
qga/commands.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qga/commands.c b/qga/commands.c
index ce172edd2d..88c1c99fe5 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -154,7 +154,7 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error
**errp)
gei = guest_exec_info_find(pid);
if (gei == NULL) {
-error_setg(errp, QERR_INVALID_PARAMETER, "pid");
+error_setg(errp, "PID " PRId64 " does not exist");
return NULL;
}
--
2.41.0
[PULL 5/7] net: Fix a misleading error message
The error message
$ qemu-system-x86_64 -netdev user,id=net0,ipv6-net=fec0::0/
qemu-system-x86_64: -netdev user,id=net0,ipv6-net=fec0::0/: Parameter
'ipv6-prefixlen' expects a number
points to ipv6-prefixlen instead of ipv6-net. Fix:
qemu-system-x86_64: -netdev user,id=net0,ipv6-net=fec0::0/: parameter
'ipv6-net' expects a number after '/'
Signed-off-by: Markus Armbruster
Message-ID: <2023103059.3407803-6-arm...@redhat.com>
---
net/net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/net.c b/net/net.c
index c0c0cbe99e..8e67a20abc 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1227,7 +1227,7 @@ static int net_client_init(QemuOpts *opts, bool
is_netdev, Error **errp)
int ret = -1;
Visitor *v = opts_visitor_new(opts);
-/* Parse convenience option format ip6-net=fec0::0[/64] */
+/* Parse convenience option format ipv6-net=fec0::0[/64] */
const char *ip6_net = qemu_opt_get(opts, "ipv6-net");
if (ip6_net) {
@@ -1247,8 +1247,8 @@ static int net_client_init(QemuOpts *opts, bool
is_netdev, Error **errp)
if (substrings[1] &&
qemu_strtoul(substrings[1], NULL, 10, _len))
{
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "ipv6-prefixlen", "a number");
+error_setg(errp,
+ "parameter 'ipv6-net' expects a number after '/'");
goto out;
}
--
2.41.0
[PULL 4/7] ui/qmp-cmds: Improve two error messages
set_password with "protocol": "vnc" supports only "connected": "keep".
Any other value is rejected with
Invalid parameter 'connected'
Improve this to
parameter 'connected' must be 'keep' when 'protocol' is 'vnc'
client_migrate_info requires "port" or "tls-port". When both are
missing, it fails with
Parameter 'port/tls-port' is missing
Improve this to
parameter 'port' or 'tls-port' is required
Signed-off-by: Markus Armbruster
Message-ID: <2023103059.3407803-5-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé
---
ui/ui-qmp-cmds.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/ui/ui-qmp-cmds.c b/ui/ui-qmp-cmds.c
index d772e1cb7f..74fa6c6ec5 100644
--- a/ui/ui-qmp-cmds.c
+++ b/ui/ui-qmp-cmds.c
@@ -44,7 +44,8 @@ void qmp_set_password(SetPasswordOptions *opts, Error **errp)
assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
if (opts->connected != SET_PASSWORD_ACTION_KEEP) {
/* vnc supports "connected=keep" only */
-error_setg(errp, QERR_INVALID_PARAMETER, "connected");
+error_setg(errp, "parameter 'connected' must be 'keep'"
+ " when 'protocol' is 'vnc'");
return;
}
/*
@@ -195,7 +196,7 @@ void qmp_client_migrate_info(const char *protocol, const
char *hostname,
}
if (!has_port && !has_tls_port) {
-error_setg(errp, QERR_MISSING_PARAMETER, "port/tls-port");
+error_setg(errp, "parameter 'port' or 'tls-port' is required");
return;
}
--
2.41.0
[PULL 6/7] balloon: Fix a misleading error message
The error message
{"execute": "balloon", "arguments":{"value": -1}}
{"error": {"class": "GenericError", "desc": "Parameter 'target' expects a
size"}}
points to 'target' instead of 'value'. Fix:
{"error": {"class": "GenericError", "desc": "Parameter 'value' expects a
size"}}
Root cause: qmp_balloon()'s parameter is named @target. Rename it to
@value to match the QAPI schema.
Signed-off-by: Markus Armbruster
Message-ID: <2023103059.3407803-7-arm...@redhat.com>
Reviewed-by: David Hildenbrand
Reviewed-by: Michael S. Tsirkin
Tested-by: Mario Casquero
---
system/balloon.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/system/balloon.c b/system/balloon.c
index e0e8969a4b..fda7af832e 100644
--- a/system/balloon.c
+++ b/system/balloon.c
@@ -90,17 +90,17 @@ BalloonInfo *qmp_query_balloon(Error **errp)
return info;
}
-void qmp_balloon(int64_t target, Error **errp)
+void qmp_balloon(int64_t value, Error **errp)
{
if (!have_balloon(errp)) {
return;
}
-if (target <= 0) {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "target", "a size");
+if (value <= 0) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "value", "a size");
return;
}
-trace_balloon_event(balloon_opaque, target);
-balloon_event_fn(balloon_opaque, target);
+trace_balloon_event(balloon_opaque, value);
+balloon_event_fn(balloon_opaque, value);
}
--
2.41.0
[PATCH] docs/devel: Add VFIO iommufd backend documentation
Suggested-by: Cédric Le Goater Signed-off-by: Zhenzhong Duan --- MAINTAINERS| 1 + docs/devel/index-internals.rst | 1 + docs/devel/vfio-iommufd.rst| 115 + 3 files changed, 117 insertions(+) create mode 100644 docs/devel/vfio-iommufd.rst diff --git a/MAINTAINERS b/MAINTAINERS index d86ba56a49..07990456ed 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2175,6 +2175,7 @@ F: backends/iommufd.c F: include/sysemu/iommufd.h F: include/qemu/chardev_open.h F: util/chardev_open.c +F: docs/devel/vfio-iommufd.rst vhost M: Michael S. Tsirkin diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst index 6f81df92bc..3def4a138b 100644 --- a/docs/devel/index-internals.rst +++ b/docs/devel/index-internals.rst @@ -18,5 +18,6 @@ Details about QEMU's various subsystems including how to add features to them. s390-dasd-ipl tracing vfio-migration + vfio-iommufd writing-monitor-commands virtio-backends diff --git a/docs/devel/vfio-iommufd.rst b/docs/devel/vfio-iommufd.rst new file mode 100644 index 00..59804a7f26 --- /dev/null +++ b/docs/devel/vfio-iommufd.rst @@ -0,0 +1,115 @@ +=== +IOMMUFD BACKEND usage with VFIO +=== + +(Same meaning for backend/container/BE) + +With the introduction of iommufd, the Linux kernel provides a generic +interface for user space drivers to propagate their DMA mappings to kernel +for assigned devices. While the legacy kernel interface is group-centric, +the new iommufd interface is device-centric, relying on device fd and iommufd. + +To support both interfaces in the QEMU VFIO device, introduce a base container +to abstract the common part of VFIO legacy and iommufd container. So that the +generic VFIO code can use either container. + +The base container implements generic functions such as memory_listener and +address space management whereas the derived container implements callbacks +specific to either legacy or iommufd. Each container has its own way to setup +secure context and dma management interface. The below diagram shows how it +looks like with both containers. + +VFIO AddressSpace/Memory ++---+ +--+ +-+ +-+ +| pci | | platform | | ap | | ccw | ++---+---+ ++-+ +--+--+ +--+--+ +--+ +| | ||| AddressSpace | +| | ||++-+ ++---V---V---VV+ / +| VFIOAddressSpace | <+ +| | | MemoryListener +|VFIOContainerBase list | ++---+++ +|| +|| ++---V--++V--+ +| iommufd||vfio legacy| +| container || container | ++---+--+++--+ +|| +| /dev/iommu | /dev/vfio/vfio +| /dev/vfio/devices/vfioX| /dev/vfio/$group_id +Userspace || +++=== +Kernel | device fd | ++---+| group/container fd +| (BIND_IOMMUFD || (SET_CONTAINER/SET_IOMMU) +| ATTACH_IOAS) || device fd +| || +| +---VV-+ +iommufd | |vfio | +(map/unmap | +-++---+ +ioas_copy) | || map/unmap +| || + +--V--++-V--+ +--V+ + | iommfd core || device| | vfio iommu | + +-+++ +---+ + +[Secure Context setup] +- iommufd BE: uses device fd and iommufd to setup secure context + (bind_iommufd, attach_ioas) +- vfio legacy BE: uses group fd and container fd to setup secure context + (set_container, set_iommu) + +[Device access] +- iommufd BE: device fd is opened through /dev/vfio/devices/vfioX +- vfio legacy BE: device fd is retrieved from group fd ioctl + +[DMA Mapping flow] +1. VFIOAddressSpace receives MemoryRegion add/del via MemoryListener +2. VFIO populates DMA map/unmap via the container BEs + *) iommufd BE: uses iommufd + *) vfio legacy BE: uses container fd + + +Example configuration += + +Step 1: configure the host device +- + +It's
RE: [PATCH v5 10/11] hw/net: GMAC Tx Implementation
-Original Message-
From: Nabih Estefan
Sent: Saturday, October 28, 2023 1:56 AM
To: peter.mayd...@linaro.org
Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; CS20 KFTing
; wuhao...@google.com; jasonw...@redhat.com; IS20 Avi
Fishman ; nabiheste...@google.com; CS20 KWLiu
; IS20 Tomer Maimon ; IN20 Hila
Miranda-Kuzi
Subject: [PATCH v5 10/11] hw/net: GMAC Tx Implementation
From: Nabih Estefan Diaz
- Implementation of Transmit function for packets
- Implementation for reading and writing from and to descriptors in
memory for Tx
NOTE: This function implements the steps detailed in the datasheet for
transmitting messages from the GMAC.
Change-Id: I5f6148eaf8548726a48db9d4a6d8796bb7ed12dc
Signed-off-by: Nabih Estefan
---
hw/net/npcm_gmac.c | 151
hw/net/trace-events | 1 -
2 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c index
cd59ca5fd4..0098b00a49 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -267,6 +267,7 @@ static int gmac_write_tx_desc(dma_addr_t addr, struct
NPCMGMACTxDesc *desc)
}
return 0;
}
+
static int gmac_rx_transfer_frame_to_buffer(uint32_t rx_buf_len,
uint32_t *left_frame,
uint32_t rx_buf_addr, @@ -488,6
+489,156 @@ static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf,
size_t len)
gmac->regs[R_NPCM_DMA_HOST_RX_DESC] = desc_addr;
return len;
}
+
+static int gmac_tx_get_csum(uint32_t tdes1) {
+uint32_t mask = TX_DESC_TDES1_CHKSM_INS_CTRL_MASK(tdes1);
+int csum = 0;
+
+if (likely(mask > 0)) {
+csum |= CSUM_IP;
+}
+if (likely(mask > 1)) {
+csum |= CSUM_TCP | CSUM_UDP;
+}
+
+return csum;
+}
+
+static void gmac_try_send_next_packet(NPCMGMACState *gmac) {
+/*
+ * Comments about steps refer to steps for
+ * transmitting in page 384 of datasheet
+ */
+uint16_t tx_buffer_size = 2048;
+g_autofree uint8_t *tx_send_buffer = g_malloc(tx_buffer_size);
+uint32_t desc_addr;
+struct NPCMGMACTxDesc tx_desc;
+uint32_t tx_buf_addr, tx_buf_len;
+uint16_t length = 0;
+uint8_t *buf = tx_send_buffer;
+uint32_t prev_buf_size = 0;
+int csum = 0;
+
+/* steps 1&2 */
+if (!gmac->regs[R_NPCM_DMA_HOST_TX_DESC]) {
+gmac->regs[R_NPCM_DMA_HOST_TX_DESC] =
+NPCM_DMA_HOST_TX_DESC_MASK(gmac->regs[R_NPCM_DMA_TX_BASE_ADDR]);
+}
+desc_addr = gmac->regs[R_NPCM_DMA_HOST_TX_DESC];
+
+while (true) {
+gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+NPCM_DMA_STATUS_TX_RUNNING_FETCHING_STATE);
+if (gmac_read_tx_desc(desc_addr, _desc)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "TX Descriptor @ 0x%x can't be read\n",
+ desc_addr);
+return;
+}
+/* step 3 */
+
+trace_npcm_gmac_packet_desc_read(DEVICE(gmac)->canonical_path,
+desc_addr);
+trace_npcm_gmac_debug_desc_data(DEVICE(gmac)->canonical_path, _desc,
+tx_desc.tdes0, tx_desc.tdes1, tx_desc.tdes2,
+ tx_desc.tdes3);
+
+/* 1 = DMA Owned, 0 = Software Owned */
+if (!(tx_desc.tdes0 & TX_DESC_TDES0_OWN)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "TX Descriptor @ 0x%x is owned by software\n",
+ desc_addr);
+gmac->regs[R_NPCM_DMA_STATUS] |= NPCM_DMA_STATUS_TU;
+gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+NPCM_DMA_STATUS_TX_SUSPENDED_STATE);
+gmac_update_irq(gmac);
+return;
+}
+
+gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+NPCM_DMA_STATUS_TX_RUNNING_READ_STATE);
+/* Give the descriptor back regardless of what happens. */
+tx_desc.tdes0 &= ~TX_DESC_TDES0_OWN;
+
+if (tx_desc.tdes1 & TX_DESC_TDES1_FIRST_SEG_MASK) {
+csum = gmac_tx_get_csum(tx_desc.tdes1);
+}
+
+/* step 4 */
+tx_buf_addr = tx_desc.tdes2;
+gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
+tx_buf_len = TX_DESC_TDES1_BFFR1_SZ_MASK(tx_desc.tdes1);
+buf = _send_buffer[prev_buf_size];
+
+if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+tx_buffer_size = prev_buf_size + tx_buf_len;
+tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
+buf = _send_buffer[prev_buf_size];
+}
+
+/* step 5 */
+if (dma_memory_read(_space_memory, tx_buf_addr, buf,
+tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read packet @
0x%x\n",
+__func__, tx_buf_addr);
+return;
+}
+length += tx_buf_len;
+
Re: [PATCH 0/7] Miscellaneous error message improvements
Queued.
RE: [PATCH v5 09/11] hw/net: GMAC Rx Implementation
-Original Message-
From: Nabih Estefan
Sent: Saturday, October 28, 2023 1:56 AM
To: peter.mayd...@linaro.org
Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; CS20 KFTing
; wuhao...@google.com; jasonw...@redhat.com; IS20 Avi
Fishman ; nabiheste...@google.com; CS20 KWLiu
; IS20 Tomer Maimon ; IN20 Hila
Miranda-Kuzi
Subject: [PATCH v5 09/11] hw/net: GMAC Rx Implementation
From: Nabih Estefan Diaz
- Implementation of Receive function for packets
- Implementation for reading and writing from and to descriptors in
memory for Rx
When RX starts, we need to flush the queued packets so that they can be
received by the GMAC device. Without this it won't work with TAP NIC device.
When RX descriptor list is full, it returns a DMA_STATUS for software to handle
it. But there's no way to indicate the software ha handled all RX descriptors
and the whole pipeline stalls.
We do something similar to NPCM7XX EMC to handle this case.
1. Return packet size when RX descriptor is full, effectively dropping these
packets in such a case.
2. When software clears RX descriptor full bit, continue receiving further
packets by flushing QEMU packet queue.
Change-Id: Ie176cbbeecbc02d3bb7442ef6262c9ef8029c232
Signed-off-by: Hao Wu
Signed-off-by: Nabih Estefan
---
hw/net/npcm_gmac.c | 331 -
include/hw/net/npcm_gmac.h | 28 ++--
2 files changed, 343 insertions(+), 16 deletions(-)
diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c index
220955346c..cd59ca5fd4 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -23,7 +23,11 @@
#include "hw/registerfields.h"
#include "hw/net/mii.h"
#include "hw/net/npcm_gmac.h"
+#include "linux/if_ether.h"
#include "migration/vmstate.h"
+#include "net/checksum.h"
+#include "net/net.h"
+#include "qemu/cutils.h"
#include "qemu/log.h"
#include "qemu/units.h"
#include "sysemu/dma.h"
@@ -146,6 +150,17 @@ static void gmac_phy_set_link(NPCMGMACState *s, bool
active)
static bool gmac_can_receive(NetClientState *nc) {
+NPCMGMACState *gmac = NPCM_GMAC(qemu_get_nic_opaque(nc));
+
+/* If GMAC receive is disabled. */
+if (!(gmac->regs[R_NPCM_GMAC_MAC_CONFIG] & NPCM_GMAC_MAC_CONFIG_RX_EN)) {
+return false;
+}
+
+/* If GMAC DMA RX is stopped. */
+if (!(gmac->regs[R_NPCM_DMA_CONTROL] & NPCM_DMA_CONTROL_START_STOP_RX)) {
+return false;
+}
return true;
}
@@ -191,11 +206,288 @@ static void gmac_update_irq(NPCMGMACState *gmac)
qemu_set_irq(gmac->irq, level);
}
-static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf, size_t len)
+static int gmac_read_rx_desc(dma_addr_t addr, struct NPCMGMACRxDesc
+*desc) {
+if (dma_memory_read(_space_memory, addr, desc,
+sizeof(*desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read descriptor @ 0x%"
+ HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
+desc->rdes0 = le32_to_cpu(desc->rdes0);
+desc->rdes1 = le32_to_cpu(desc->rdes1);
+desc->rdes2 = le32_to_cpu(desc->rdes2);
+desc->rdes3 = le32_to_cpu(desc->rdes3);
+return 0;
+}
+
+static int gmac_write_rx_desc(dma_addr_t addr, struct NPCMGMACRxDesc
+*desc) {
+struct NPCMGMACRxDesc le_desc;
+le_desc.rdes0 = cpu_to_le32(desc->rdes0);
+le_desc.rdes1 = cpu_to_le32(desc->rdes1);
+le_desc.rdes2 = cpu_to_le32(desc->rdes2);
+le_desc.rdes3 = cpu_to_le32(desc->rdes3);
+if (dma_memory_write(_space_memory, addr, _desc,
+sizeof(le_desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to write descriptor @ 0x%"
+ HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
+return 0;
+}
+
+static int gmac_read_tx_desc(dma_addr_t addr, struct NPCMGMACTxDesc
+*desc) {
+if (dma_memory_read(_space_memory, addr, desc,
+sizeof(*desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read descriptor @ 0x%"
+ HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
+desc->tdes0 = le32_to_cpu(desc->tdes0);
+desc->tdes1 = le32_to_cpu(desc->tdes1);
+desc->tdes2 = le32_to_cpu(desc->tdes2);
+desc->tdes3 = le32_to_cpu(desc->tdes3);
+return 0;
+}
+
+static int gmac_write_tx_desc(dma_addr_t addr, struct NPCMGMACTxDesc
+*desc)
{
-/* Placeholder */
+struct NPCMGMACTxDesc le_desc;
+le_desc.tdes0 = cpu_to_le32(desc->tdes0);
+le_desc.tdes1 = cpu_to_le32(desc->tdes1);
+le_desc.tdes2 = cpu_to_le32(desc->tdes2);
+le_desc.tdes3 = cpu_to_le32(desc->tdes3);
+if (dma_memory_write(_space_memory, addr, _desc,
+sizeof(le_desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to write descriptor @ 0x%"
+ HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
return 0;
}
+static int
Re: QEMU snapshotting
On 231115 1522, Brian Cain wrote: > Alexander, Bandan, Paolo, Stefan, Manuel, > > Hi, I'm Brian and I maintain the Hexagon arch for QEMU. Elia, a security > researcher at Qualcomm is exploring ways to fuzz some hexagon OS kernel with > QEMU and in particular leveraging snapshotting, inspired by your research and > more. I'm not an expert on the details, but I'd like to make an introduction > and see if there's an opportunity for us to learn from one another. Maybe we > can have a call to kick things off? > Hi Brian, Elia, Sounds interesting! Happy to hop on a call to discuss. Mornings (EST) tend to work best for me. -Alex > -Brian
Re: [PATCH-for-8.2] target/nios2: Deprecate the Nios II architecture
On 17/11/2023 08.02, Philippe Mathieu-Daudé wrote:
See commit 9ba1caf510 ("MAINTAINERS: Mark the Nios II CPU as orphan"),
last contribution from Chris was in 2012 [1] and Marek in 2018 [2].
[1]
https://lore.kernel.org/qemu-devel/1352607539-10455-2-git-send-email-crwu...@gmail.com/
[2]
https://lore.kernel.org/qemu-devel/805fc7b5-03f0-56d4-abfd-ed010d4fa...@denx.de/
Signed-off-by: Philippe Mathieu-Daudé
---
docs/about/deprecated.rst | 15 +++
hw/nios2/10m50_devboard.c | 1 +
hw/nios2/generic_nommu.c | 1 +
3 files changed, 17 insertions(+)
Being orphan for so long in QEMU, I guess it makes sense to mark it as
deprecated here now. We can still reconsider if a new maintainer shows up...
Reviewed-by: Thomas Huth
Re: [PATCH v2] system/memory: use ldn_he_p/stn_he_p
On 16.11.23 17:36, Patrick Venture wrote: Using direct pointer dereferencing can allow for unaligned accesses, which was seen during execution with sanitizers enabled. Reviewed-by: Chris Rauer Reviewed-by: Peter Foley Signed-off-by: Patrick Venture Cc: qemu-sta...@nongnu.org Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
[PATCH] riscv: Fix SiFive E CLINT clock frequency
If you check the manual of SiFive E310 (https://cdn.sparkfun.com/assets/7/f/0/2/7/fe310-g002-manual-v19p05.pdf), you can see in Figure 1 that the CLINT is connected to the real time clock, which also feeds the AON peripheral (they share the same clock). In page 43, the docs also say that the timer registers of the CLINT count ticks from the rtcclk. I am currently playing with bare metal applications both in QEMU and a physical SiFive E310 board and I confirm that the CLINT clock in the physical board runs at 32.768 kHz. In QEMU, the same app produces a completely different outcome, as sometimes a new CLINT interrupt is triggered before finishing other tasks. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1978 Signed-off-by: Román Cárdenas --- hw/riscv/sifive_e.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 0d37adc542..87d9602383 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -225,7 +225,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp) RISCV_ACLINT_SWI_SIZE, RISCV_ACLINT_DEFAULT_MTIMER_SIZE, 0, ms->smp.cpus, RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME, -RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, false); +SIFIVE_E_LFCLK_DEFAULT_FREQ, false); sifive_e_prci_create(memmap[SIFIVE_E_DEV_PRCI].base); /* AON */ -- 2.39.3 (Apple Git-145)
[PATCH] hw/usb: fix xhci port notify
>From MCF5253 Reference manual
>https://www.nxp.com/docs/en/reference-manual/MCF5253RM.pdf
Host mode: The controller sets this bit to a one when on any port a Connect
Status occurs, a PortEnable/Disable Change occurs, an Over Current Change
occurs, or the Force Port Resume bit is set as theresult of a J-K transition on
the suspended port.
---
hw/usb/hcd-xhci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4b60114207..1b2f4ac721 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2627,6 +2627,7 @@ static void xhci_port_notify(XHCIPort *port, uint32_t
bits)
if (!xhci_running(port->xhci)) {
return;
}
+port->xhci->usbsts |= USBSTS_PCD;
xhci_event(port->xhci, , 0);
}
--
2.34.1
Re: [PATCH v2] system/memory: use ldn_he_p/stn_he_p
On 16/11/23 17:36, Patrick Venture wrote: Using direct pointer dereferencing can allow for unaligned accesses, which was seen during execution with sanitizers enabled. Reviewed-by: Chris Rauer Reviewed-by: Peter Foley Signed-off-by: Patrick Venture Cc: qemu-sta...@nongnu.org --- v2: changed commit mesage to be more accurate and switched from using memcpy to using the endian appropriate assignment load and store. --- system/memory.c | 32 ++-- 1 file changed, 2 insertions(+), 30 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
