Re: [PATCH v6 01/19] configure, meson: override C compiler for cmake
On 2/18/22 15:49, Jag Raman wrote:
Concerning the generated files, I see the following in
CMakeMesonToolchainFile.cmake:
Without patch: set(CMAKE_C_COMPILER "/opt/rh/devtoolset-9/root/usr/bin/cc" "-m64"
"-mcx16”)
With patch: set(CMAKE_C_COMPILER "cc" "-m64" "-mcx16")
I don't understand why it works at all with the latter, but the right solution
could be
set(CMAKE_C_COMPILER "/opt/rh/devtoolset-9/root/usr/bin/cc")
set(CMAKE_C_COMPILER_ARG1 "-m64")
set(CMAKE_C_COMPILER_ARG2 "-mcx16")
Perhaps you can try the following patch to meson (patch it in qemu's build
directory and make sure to use --meson=internal):
diff --git a/mesonbuild/cmake/toolchain.py b/mesonbuild/cmake/toolchain.py
index 316f57cb5..9756864ee 100644
--- a/mesonbuild/cmake/toolchain.py
+++ b/mesonbuild/cmake/toolchain.py
@@ -191,11 +191,14 @@ class CMakeToolchain:
continue
if len(exe_list) >= 2 and not self.is_cmdline_option(comp_obj, exe_list[1]):
-defaults[prefix + 'COMPILER_LAUNCHER'] =
[make_abs(exe_list[0])]
+defaults[f'{prefix}COMPILER_LAUNCHER'] =
[make_abs(exe_list[0])]
exe_list = exe_list[1:]
exe_list[0] = make_abs(exe_list[0])
-defaults[prefix + 'COMPILER'] = exe_list
+defaults[f'{prefix}COMPILER'] = [exe_list[0]]
+for i in range(1, len(exe_list)):
+defaults[f'{prefix}COMPILER_ARG{i}'] = [exe_list[i]]
+
if comp_obj.get_id() == 'clang-cl':
defaults['CMAKE_LINKER'] = comp_obj.get_linker_exelist()
Thanks,
Paolo
[PATCH v10 0/5] QEMU RISC-V AIA support
From: Anup Patel The advanced interrupt architecture (AIA) extends the per-HART local interrupt support. Along with this, it also adds IMSIC (MSI contrllor) and Advanced PLIC (wired interrupt controller). The latest AIA draft specification can be found here: https://github.com/riscv/riscv-aia/releases/download/0.2-draft.28/riscv-interrupts-028.pdf This series adds RISC-V AIA support in QEMU which includes emulating all AIA local CSRs, APLIC, and IMSIC. Only AIA local interrupt filtering is not implemented because we don't have any local interrupt greater than 12. To enable AIA in QEMU, use one of the following: 1) Only AIA local interrupt CSRs: Pass "x-aia=true" as CPU paramenter in the QEMU command-line 2) Only APLIC for virt machine: Pass "aia=aplic" as machine parameter in the QEMU command-line 3) Both APLIC and IMSIC for virt machine: Pass "aia=aplic-imsic" as machine parameter in the QEMU command-line 4) Both APLIC and IMSIC with 2 guest files for virt machine: Pass "aia=aplic-imsic,aia-guests=2" as machine parameter in the QEMU command-line To test series, we require Linux with AIA support which can be found in: riscv_aia_v1 branch at https://github.com/avpatel/linux.git This series can be found riscv_aia_v10 branch at: https://github.com/avpatel/qemu.git Changes since v9: - Rebased on latest riscv-to-apply.next branch of Alistair's repo - Removed first 18 PATCHs since these are already merged - Fixed 32-bit system compile error in PATCH3 Changes since v8: - Use error_setg() in riscv_imsic_realize() added by PATCH20 Changes since v7: - Rebased on latest riscv-to-apply.next branch of Alistair's repo - Improved default priority assignment in PATCH9 Changes since v6: - Fixed priority comparison in riscv_cpu_pending_to_irq() of PATCH9 - Fixed typos in comments added by PATCH11 - Added "pend = true;" for CSR_MSETEIPNUM case of rmw_xsetclreinum() in PATCH15 - Handle ithreshold == 0 case in riscv_aplic_idc_topi() of PATCH18 - Allow setting pending bit for Level0 or Level1 interrupts in riscv_aplic_set_pending() of PATCH18 - Force DOMAINCFG[31:24] bits to 0x80 in riscv_aplic_read() of PATCH18 - For APLIC direct mode, set target.iprio to 1 when zero is writtern in PATCH18 - Handle eithreshold == 0 case in riscv_imsic_topei() of PATCH20 Changes since v5: - Moved VSTOPI_NUM_SRCS define to top of the file in PATCH13 - Fixed typo in PATCH16 Changes since v4: - Changed IRQ_LOCAL_MAX to 16 in PATCH2 - Fixed typo in PATCH10 - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH11 - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH14 - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH15 - Replaced TARGET_LONG_BITS with xlen passed via ireg callback in PATCH20 - Retrict maximum IMSIC guest files per-HART of virt machine to 7 in PATCH21. - Added separate PATCH23 to increase maximum number of allowed CPUs for virt machine Changes since v3: - Replaced "aplic,xyz" and "imsic,xyz" DT properties with "riscv,xyz" DT properties because "aplic" and "imsic" are not valid vendor names required by Linux DT schema checker. Changes since v2: - Update PATCH4 to check and inject interrupt after V=1 when transitioning from V=0 to V=1 Changes since v1: - Revamped whole series and created more granular patches - Added HGEIE and HGEIP CSR emulation for H-extension - Added APLIC emulation - Added IMSIC emulation Anup Patel (5): hw/riscv: virt: Add optional AIA APLIC support to virt machine hw/intc: Add RISC-V AIA IMSIC device emulation hw/riscv: virt: Add optional AIA IMSIC support to virt machine docs/system: riscv: Document AIA options for virt machine hw/riscv: virt: Increase maximum number of allowed CPUs docs/system/riscv/virt.rst| 16 + hw/intc/Kconfig | 3 + hw/intc/meson.build | 1 + hw/intc/riscv_imsic.c | 448 ++ hw/riscv/Kconfig | 2 + hw/riscv/virt.c | 698 -- include/hw/intc/riscv_imsic.h | 68 include/hw/riscv/virt.h | 41 +- 8 files changed, 1156 insertions(+), 121 deletions(-) create mode 100644 hw/intc/riscv_imsic.c create mode 100644 include/hw/intc/riscv_imsic.h -- 2.25.1
[PATCH v10 3/5] hw/riscv: virt: Add optional AIA IMSIC support to virt machine
From: Anup Patel
We extend virt machine to emulate both AIA IMSIC and AIA APLIC
devices only when "aia=aplic-imsic" parameter is passed along
with machine name in the QEMU command-line. The AIA IMSIC is
only a per-HART MSI controller so we use AIA APLIC in MSI-mode
to forward all wired interrupts as MSIs to the AIA IMSIC.
We also provide "aia-guests=" parameter which can be used
to specify number of VS-level AIA IMSIC Guests MMIO pages for
each HART.
Signed-off-by: Anup Patel
Signed-off-by: Anup Patel
Acked-by: Alistair Francis
---
hw/riscv/Kconfig| 1 +
hw/riscv/virt.c | 439
include/hw/riscv/virt.h | 17 +-
3 files changed, 373 insertions(+), 84 deletions(-)
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index c30bb7cb6c..91bb9d21c4 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -43,6 +43,7 @@ config RISCV_VIRT
select SERIAL
select RISCV_ACLINT
select RISCV_APLIC
+select RISCV_IMSIC
select SIFIVE_PLIC
select SIFIVE_TEST
select VIRTIO_MMIO
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 6b06f79b46..94fbf63ec8 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -34,6 +34,7 @@
#include "hw/riscv/numa.h"
#include "hw/intc/riscv_aclint.h"
#include "hw/intc/riscv_aplic.h"
+#include "hw/intc/riscv_imsic.h"
#include "hw/intc/sifive_plic.h"
#include "hw/misc/sifive_test.h"
#include "chardev/char.h"
@@ -44,6 +45,18 @@
#include "hw/pci-host/gpex.h"
#include "hw/display/ramfb.h"
+#define VIRT_IMSIC_GROUP_MAX_SIZE (1U << IMSIC_MMIO_GROUP_MIN_SHIFT)
+#if VIRT_IMSIC_GROUP_MAX_SIZE < \
+IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS)
+#error "Can't accomodate single IMSIC group in address space"
+#endif
+
+#define VIRT_IMSIC_MAX_SIZE(VIRT_SOCKETS_MAX * \
+VIRT_IMSIC_GROUP_MAX_SIZE)
+#if 0x400 < VIRT_IMSIC_MAX_SIZE
+#error "Can't accomodate all IMSIC groups in address space"
+#endif
+
static const MemMapEntry virt_memmap[] = {
[VIRT_DEBUG] = {0x0, 0x100 },
[VIRT_MROM] ={ 0x1000,0xf000 },
@@ -59,6 +72,8 @@ static const MemMapEntry virt_memmap[] = {
[VIRT_VIRTIO] = { 0x10001000,0x1000 },
[VIRT_FW_CFG] = { 0x1010, 0x18 },
[VIRT_FLASH] = { 0x2000, 0x400 },
+[VIRT_IMSIC_M] = { 0x2400, VIRT_IMSIC_MAX_SIZE },
+[VIRT_IMSIC_S] = { 0x2800, VIRT_IMSIC_MAX_SIZE },
[VIRT_PCIE_ECAM] = { 0x3000,0x1000 },
[VIRT_PCIE_MMIO] = { 0x4000,0x4000 },
[VIRT_DRAM] ={ 0x8000, 0x0 },
@@ -310,7 +325,7 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
{
int cpu;
char *name;
-unsigned long addr;
+unsigned long addr, size;
uint32_t aclint_cells_size;
uint32_t *aclint_mswi_cells;
uint32_t *aclint_sswi_cells;
@@ -331,29 +346,38 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
}
aclint_cells_size = s->soc[socket].num_harts * sizeof(uint32_t) * 2;
-addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
-name = g_strdup_printf("/soc/mswi@%lx", addr);
-qemu_fdt_add_subnode(mc->fdt, name);
-qemu_fdt_setprop_string(mc->fdt, name, "compatible", "riscv,aclint-mswi");
-qemu_fdt_setprop_cells(mc->fdt, name, "reg",
-0x0, addr, 0x0, RISCV_ACLINT_SWI_SIZE);
-qemu_fdt_setprop(mc->fdt, name, "interrupts-extended",
-aclint_mswi_cells, aclint_cells_size);
-qemu_fdt_setprop(mc->fdt, name, "interrupt-controller", NULL, 0);
-qemu_fdt_setprop_cell(mc->fdt, name, "#interrupt-cells", 0);
-riscv_socket_fdt_write_id(mc, mc->fdt, name, socket);
-g_free(name);
+if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
+addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
+name = g_strdup_printf("/soc/mswi@%lx", addr);
+qemu_fdt_add_subnode(mc->fdt, name);
+qemu_fdt_setprop_string(mc->fdt, name, "compatible",
+"riscv,aclint-mswi");
+qemu_fdt_setprop_cells(mc->fdt, name, "reg",
+0x0, addr, 0x0, RISCV_ACLINT_SWI_SIZE);
+qemu_fdt_setprop(mc->fdt, name, "interrupts-extended",
+aclint_mswi_cells, aclint_cells_size);
+qemu_fdt_setprop(mc->fdt, name, "interrupt-controller", NULL, 0);
+qemu_fdt_setprop_cell(mc->fdt, name, "#interrupt-cells", 0);
+riscv_socket_fdt_write_id(mc, mc->fdt, name, socket);
+g_free(name);
+}
-addr = memmap[VIRT_CLINT].base + RISCV_ACLINT_SWI_SIZE +
-(memmap[VIRT_CLINT].size * socket);
+if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
+addr = memmap[VIRT_CLINT].base +
+ (RISCV_ACLINT_DEFAULT_MTIMER_SIZE * socket);
+size = RISCV_ACLINT_DEFAULT_MTIMER_SIZE;
+} else {
+addr = memmap[VIRT_CLINT].base + RISCV_ACLINT_SWI_SIZE +
+
[PATCH v10 1/5] hw/riscv: virt: Add optional AIA APLIC support to virt machine
From: Anup Patel
We extend virt machine to emulate AIA APLIC devices only when
"aia=aplic" parameter is passed along with machine name in QEMU
command-line. When "aia=none" or not specified then we fallback
to original PLIC device emulation.
Signed-off-by: Anup Patel
Signed-off-by: Anup Patel
Reviewed-by: Alistair Francis
---
hw/riscv/Kconfig| 1 +
hw/riscv/virt.c | 291
include/hw/riscv/virt.h | 26 +++-
3 files changed, 259 insertions(+), 59 deletions(-)
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index d2d869aaad..c30bb7cb6c 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -42,6 +42,7 @@ config RISCV_VIRT
select PFLASH_CFI01
select SERIAL
select RISCV_ACLINT
+select RISCV_APLIC
select SIFIVE_PLIC
select SIFIVE_TEST
select VIRTIO_MMIO
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index e3068d6126..6b06f79b46 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -33,6 +33,7 @@
#include "hw/riscv/boot.h"
#include "hw/riscv/numa.h"
#include "hw/intc/riscv_aclint.h"
+#include "hw/intc/riscv_aplic.h"
#include "hw/intc/sifive_plic.h"
#include "hw/misc/sifive_test.h"
#include "chardev/char.h"
@@ -52,6 +53,8 @@ static const MemMapEntry virt_memmap[] = {
[VIRT_ACLINT_SSWI] = { 0x2F0,0x4000 },
[VIRT_PCIE_PIO] ={ 0x300, 0x1 },
[VIRT_PLIC] ={ 0xc00, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) },
+[VIRT_APLIC_M] = { 0xc00, APLIC_SIZE(VIRT_CPUS_MAX) },
+[VIRT_APLIC_S] = { 0xd00, APLIC_SIZE(VIRT_CPUS_MAX) },
[VIRT_UART0] = { 0x1000, 0x100 },
[VIRT_VIRTIO] = { 0x10001000,0x1000 },
[VIRT_FW_CFG] = { 0x1010, 0x18 },
@@ -133,12 +136,13 @@ static void virt_flash_map(RISCVVirtState *s,
sysmem);
}
-static void create_pcie_irq_map(void *fdt, char *nodename,
-uint32_t plic_phandle)
+static void create_pcie_irq_map(RISCVVirtState *s, void *fdt, char *nodename,
+uint32_t irqchip_phandle)
{
int pin, dev;
-uint32_t
-full_irq_map[GPEX_NUM_IRQS * GPEX_NUM_IRQS * FDT_INT_MAP_WIDTH] = {};
+uint32_t irq_map_stride = 0;
+uint32_t full_irq_map[GPEX_NUM_IRQS * GPEX_NUM_IRQS *
+ FDT_MAX_INT_MAP_WIDTH] = {};
uint32_t *irq_map = full_irq_map;
/* This code creates a standard swizzle of interrupts such that
@@ -156,23 +160,31 @@ static void create_pcie_irq_map(void *fdt, char *nodename,
int irq_nr = PCIE_IRQ + ((pin + PCI_SLOT(devfn)) % GPEX_NUM_IRQS);
int i = 0;
+/* Fill PCI address cells */
irq_map[i] = cpu_to_be32(devfn << 8);
-
i += FDT_PCI_ADDR_CELLS;
-irq_map[i] = cpu_to_be32(pin + 1);
+/* Fill PCI Interrupt cells */
+irq_map[i] = cpu_to_be32(pin + 1);
i += FDT_PCI_INT_CELLS;
-irq_map[i++] = cpu_to_be32(plic_phandle);
-i += FDT_PLIC_ADDR_CELLS;
-irq_map[i] = cpu_to_be32(irq_nr);
+/* Fill interrupt controller phandle and cells */
+irq_map[i++] = cpu_to_be32(irqchip_phandle);
+irq_map[i++] = cpu_to_be32(irq_nr);
+if (s->aia_type != VIRT_AIA_TYPE_NONE) {
+irq_map[i++] = cpu_to_be32(0x4);
+}
-irq_map += FDT_INT_MAP_WIDTH;
+if (!irq_map_stride) {
+irq_map_stride = i;
+}
+irq_map += irq_map_stride;
}
}
-qemu_fdt_setprop(fdt, nodename, "interrupt-map",
- full_irq_map, sizeof(full_irq_map));
+qemu_fdt_setprop(fdt, nodename, "interrupt-map", full_irq_map,
+ GPEX_NUM_IRQS * GPEX_NUM_IRQS *
+ irq_map_stride * sizeof(uint32_t));
qemu_fdt_setprop_cells(fdt, nodename, "interrupt-map-mask",
0x1800, 0, 0, 0x7);
@@ -404,8 +416,6 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
plic_addr = memmap[VIRT_PLIC].base + (memmap[VIRT_PLIC].size * socket);
plic_name = g_strdup_printf("/soc/plic@%lx", plic_addr);
qemu_fdt_add_subnode(mc->fdt, plic_name);
-qemu_fdt_setprop_cell(mc->fdt, plic_name,
-"#address-cells", FDT_PLIC_ADDR_CELLS);
qemu_fdt_setprop_cell(mc->fdt, plic_name,
"#interrupt-cells", FDT_PLIC_INT_CELLS);
qemu_fdt_setprop_string_array(mc->fdt, plic_name, "compatible",
@@ -425,6 +435,76 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
g_free(plic_cells);
}
+static void create_fdt_socket_aia(RISCVVirtState *s,
+ const MemMapEntry *memmap, int socket,
+ uint32_t *phandle, uint32_t *intc_phandles,
+ uint32_t *aplic_phandles)
+{
+int cpu;
+char *aplic_name;
+uint32_t
[PATCH v10 2/5] hw/intc: Add RISC-V AIA IMSIC device emulation
From: Anup Patel
The RISC-V AIA (Advanced Interrupt Architecture) defines a new
interrupt controller for MSIs (message signal interrupts) called
IMSIC (Incoming Message Signal Interrupt Controller). The IMSIC
is per-HART device and also suppport virtualizaiton of MSIs using
dedicated VS-level guest interrupt files.
This patch adds device emulation for RISC-V AIA IMSIC which
supports M-level, S-level, and VS-level MSIs.
Signed-off-by: Anup Patel
Signed-off-by: Anup Patel
Reviewed-by: Frank Chang
---
hw/intc/Kconfig | 3 +
hw/intc/meson.build | 1 +
hw/intc/riscv_imsic.c | 448 ++
include/hw/intc/riscv_imsic.h | 68 ++
4 files changed, 520 insertions(+)
create mode 100644 hw/intc/riscv_imsic.c
create mode 100644 include/hw/intc/riscv_imsic.h
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 528e77b4a6..ec8d4cec29 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -73,6 +73,9 @@ config RISCV_ACLINT
config RISCV_APLIC
bool
+config RISCV_IMSIC
+bool
+
config SIFIVE_PLIC
bool
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 7466024402..5caa337654 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -51,6 +51,7 @@ specific_ss.add(when: 'CONFIG_S390_FLIC_KVM', if_true:
files('s390_flic_kvm.c'))
specific_ss.add(when: 'CONFIG_SH_INTC', if_true: files('sh_intc.c'))
specific_ss.add(when: 'CONFIG_RISCV_ACLINT', if_true: files('riscv_aclint.c'))
specific_ss.add(when: 'CONFIG_RISCV_APLIC', if_true: files('riscv_aplic.c'))
+specific_ss.add(when: 'CONFIG_RISCV_IMSIC', if_true: files('riscv_imsic.c'))
specific_ss.add(when: 'CONFIG_SIFIVE_PLIC', if_true: files('sifive_plic.c'))
specific_ss.add(when: 'CONFIG_XICS', if_true: files('xics.c'))
specific_ss.add(when: ['CONFIG_KVM', 'CONFIG_XICS'],
diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c
new file mode 100644
index 00..8615e4cc1d
--- /dev/null
+++ b/hw/intc/riscv_imsic.c
@@ -0,0 +1,448 @@
+/*
+ * RISC-V IMSIC (Incoming Message Signaled Interrupt Controller)
+ *
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see
[PATCH v10 4/5] docs/system: riscv: Document AIA options for virt machine
From: Anup Patel We have two new machine options "aia" and "aia-guests" available for the RISC-V virt machine so let's document these options. Signed-off-by: Anup Patel Signed-off-by: Anup Patel Reviewed-by: Alistair Francis Reviewed-by: Frank Chang --- docs/system/riscv/virt.rst | 16 1 file changed, 16 insertions(+) diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst index 08ce3c4177..1272b6659e 100644 --- a/docs/system/riscv/virt.rst +++ b/docs/system/riscv/virt.rst @@ -63,6 +63,22 @@ The following machine-specific options are supported: When this option is "on", ACLINT devices will be emulated instead of SiFive CLINT. When not specified, this option is assumed to be "off". +- aia=[none|aplic|aplic-imsic] + + This option allows selecting interrupt controller defined by the AIA + (advanced interrupt architecture) specification. The "aia=aplic" selects + APLIC (advanced platform level interrupt controller) to handle wired + interrupts whereas the "aia=aplic-imsic" selects APLIC and IMSIC (incoming + message signaled interrupt controller) to handle both wired interrupts and + MSIs. When not specified, this option is assumed to be "none" which selects + SiFive PLIC to handle wired interrupts. + +- aia-guests=nnn + + The number of per-HART VS-level AIA IMSIC pages to be emulated for a guest + having AIA IMSIC (i.e. "aia=aplic-imsic" selected). When not specified, + the default number of per-HART VS-level AIA IMSIC pages is 0. + Running Linux kernel -- 2.25.1
[PATCH v10 5/5] hw/riscv: virt: Increase maximum number of allowed CPUs
From: Anup Patel To facilitate software development of RISC-V systems with large number of HARTs, we increase the maximum number of allowed CPUs to 512 (2^9). We also add a detailed source level comments about limit defines which impact the physical address space utilization. Signed-off-by: Anup Patel Signed-off-by: Anup Patel Reviewed-by: Alistair Francis Reviewed-by: Frank Chang --- hw/riscv/virt.c | 10 ++ include/hw/riscv/virt.h | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 94fbf63ec8..da50cbed43 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -45,6 +45,16 @@ #include "hw/pci-host/gpex.h" #include "hw/display/ramfb.h" +/* + * The virt machine physical address space used by some of the devices + * namely ACLINT, PLIC, APLIC, and IMSIC depend on number of Sockets, + * number of CPUs, and number of IMSIC guest files. + * + * Various limits defined by VIRT_SOCKETS_MAX_BITS, VIRT_CPUS_MAX_BITS, + * and VIRT_IRQCHIP_MAX_GUESTS_BITS are tuned for maximum utilization + * of virt machine physical address space. + */ + #define VIRT_IMSIC_GROUP_MAX_SIZE (1U << IMSIC_MMIO_GROUP_MIN_SHIFT) #if VIRT_IMSIC_GROUP_MAX_SIZE < \ IMSIC_GROUP_SIZE(VIRT_CPUS_MAX_BITS, VIRT_IRQCHIP_MAX_GUESTS_BITS) diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h index d248d0dfa0..78b058ec86 100644 --- a/include/hw/riscv/virt.h +++ b/include/hw/riscv/virt.h @@ -24,7 +24,7 @@ #include "hw/block/flash.h" #include "qom/object.h" -#define VIRT_CPUS_MAX_BITS 3 +#define VIRT_CPUS_MAX_BITS 9 #define VIRT_CPUS_MAX (1 << VIRT_CPUS_MAX_BITS) #define VIRT_SOCKETS_MAX_BITS 2 #define VIRT_SOCKETS_MAX (1 << VIRT_SOCKETS_MAX_BITS) -- 2.25.1
Re: [PATCH 01/11] mos6522: add defines for IFR bit flags
On 05/02/2022 12:06, BALATON Zoltan wrote: On Sat, 5 Feb 2022, Philippe Mathieu-Daudé wrote: On 5/2/22 11:51, Mark Cave-Ayland wrote: On 27/01/2022 23:16, BALATON Zoltan wrote: On Thu, 27 Jan 2022, Mark Cave-Ayland wrote: These are intended to make it easier to see how the physical control lines are wired for each instance. Signed-off-by: Mark Cave-Ayland --- include/hw/misc/mos6522.h | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h index fc95d22b0f..12abd8b8d2 100644 --- a/include/hw/misc/mos6522.h +++ b/include/hw/misc/mos6522.h @@ -41,13 +41,21 @@ #define IER_SET 0x80 /* set bits in IER */ #define IER_CLR 0 /* clear bits in IER */ -#define CA2_INT 0x01 -#define CA1_INT 0x02 -#define SR_INT 0x04 /* Shift register full/empty */ -#define CB2_INT 0x08 -#define CB1_INT 0x10 -#define T2_INT 0x20 /* Timer 2 interrupt */ -#define T1_INT 0x40 /* Timer 1 interrupt */ +#define CA2_INT_BIT 0 +#define CA1_INT_BIT 1 +#define SR_INT_BIT 2 /* Shift register full/empty */ +#define CB2_INT_BIT 3 +#define CB1_INT_BIT 4 +#define T2_INT_BIT 5 /* Timer 2 interrupt */ +#define T1_INT_BIT 6 /* Timer 1 interrupt */ + +#define CA2_INT (1 << CA2_INT_BIT) +#define CA1_INT (1 << CA1_INT_BIT) +#define SR_INT (1 << SR_INT_BIT) +#define CB2_INT (1 << CB2_INT_BIT) +#define CB1_INT (1 << CB1_INT_BIT) +#define T2_INT (1 << T2_INT_BIT) +#define T1_INT (1 << T1_INT_BIT) Maybe you could leave the #defines called XX_INT and then use BIT(XX_INT) instead of the second set of #defines which would provide same readability but with less #defines needed. I'm not so keen on removing the _INT defines since that would require updating all users to use BIT() everywhere which I don't think gains us much. I could certainly replace these definitions with BIT(FOO) instead of (1 << FOO) if that helps readability though. Do you mean simply doing this? -#define T1_INT 0x40 /* Timer 1 interrupt */ +#define T1_INT BIT(6) /* Timer 1 interrupt */ I meant: #define T1_INT 6 and then replace current usage of T1_INT with BIT(T1_INT) that way we don't need both T1_INT_BIT and T1_INT defines which seems redundant as BIT(T1_INT) is not much longer and still clear what it refers to. It's true that this needs more changes but the result is more readable IMO than introducing another set of defines that ome has to look up when encounters them as the meaning might not be clear. That's why I think one set of defines for bit numbers and using existing BIT(num) for values is better but it's just an idea, I don't care that much. I think the best solution here is to just use BIT() for the final shifted values like this: #define CA2_INT_BIT0 ... ... #define CA2_INTBIT(CA2_INT_BIT) Otherwise I can see there being confusion given that the BIT() macro is used for defines without a _BIT suffix which are also being used elsewhere. I'll update this in v2 accordingly. ATB, Mark.
Re: [PATCH 04/11] mos6522: switch over to use qdev gpios for IRQs
On 07/02/2022 19:29, Peter Maydell wrote:
On Thu, 27 Jan 2022 at 21:01, Mark Cave-Ayland
wrote:
For historical reasons each mos6522 instance implements its own setting and
update of the IFR flag bits using methods exposed by MOS6522DeviceClass. As
of today this is no longer required, and it is now possible to implement
the mos6522 IRQs as standard qdev gpios.
Switch over to use qdev gpios for the mos6522 device and update all instances
accordingly.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 56 +++
hw/misc/macio/cuda.c | 5 ++--
hw/misc/macio/pmu.c | 4 +--
hw/misc/mos6522.c | 15 +++
include/hw/misc/mac_via.h | 6 +
include/hw/misc/mos6522.h | 2 ++
6 files changed, 32 insertions(+), 56 deletions(-)
-static void via2_nubus_irq_request(void *opaque, int irq, int level)
+static void via2_nubus_irq_request(void *opaque, int n, int level)
{
MOS6522Q800VIA2State *v2s = opaque;
MOS6522State *s = MOS6522(v2s);
-MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s);
+qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA2_IRQ_NUBUS_BIT);
if (level) {
/* Port A nubus IRQ inputs are active LOW */
-s->a &= ~(1 << irq);
-s->ifr |= 1 << VIA2_IRQ_NUBUS_BIT;
+s->a &= ~(1 << n);
} else {
-s->a |= (1 << irq);
-s->ifr &= ~(1 << VIA2_IRQ_NUBUS_BIT);
+s->a |= (1 << n);
}
-mdc->update_irq(s);
+qemu_set_irq(irq, level);
}
It feels a bit inconsistent here that we're still reaching into
the MOS6522State to set s->a, but I guess this is still
better than what we had before.
Yeah it's a little bit messy. On the Mac the Nubus IRQs are ORd to produce the VIA
interrupt line, but the individual Nubus IRQ lines are wired to the VIA port A to
allow the state of each line to be determined. The VIA port IO lines are
bi-directional, so rather than try and implement this with GPIOs it's easiest to have
a separate VIA2 Nubus GPIOs that also handle the port IO lines as above.
-#define VIA1_IRQ_NB 8
-
#define VIA1_IRQ_ONE_SECOND (1 << VIA1_IRQ_ONE_SECOND_BIT)
#define VIA1_IRQ_60HZ (1 << VIA1_IRQ_60HZ_BIT)
#define VIA1_IRQ_ADB_READY (1 << VIA1_IRQ_ADB_READY_BIT)
@@ -42,7 +40,7 @@ struct MOS6522Q800VIA1State {
MemoryRegion via_mem;
-qemu_irq irqs[VIA1_IRQ_NB];
+qemu_irq irqs[VIA_NUM_INTS];
This irqs[] array appears to be entirely unused. You could
delete it as a separate patch before this one.
Ah yes, before this patch each VIA had its own GPIOs which used the methods in
MOS6522DeviceClass to manipulate the IFR bit. Since the Q800 VIAs have the MOS6522
device as a parent class, the GPIOs are inherited from that and so this array should
actually be removed as part of this patch.
qemu_irq auxmode_irq;
uint8_t last_b;
@@ -85,8 +83,6 @@ struct MOS6522Q800VIA1State {
#define VIA2_IRQ_SCSI_BIT CB2_INT_BIT
#define VIA2_IRQ_ASC_BITCB1_INT_BIT
-#define VIA2_IRQ_NB 8
-
#define VIA2_IRQ_SCSI_DATA (1 << VIA2_IRQ_SCSI_DATA_BIT)
#define VIA2_IRQ_NUBUS (1 << VIA2_IRQ_NUBUS_BIT)
#define VIA2_IRQ_UNUSED (1 << VIA2_IRQ_SCSI_BIT)
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index 12abd8b8d2..ced8a670bf 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -57,6 +57,8 @@
#define T2_INT (1 << T2_INT_BIT)
#define T1_INT (1 << T1_INT_BIT)
+#define VIA_NUM_INTS 5
Were we not using 5,6,7 previously ?
5 and 6 are for the in-built timers, and 7 is a set/clear flag so they are not
externally accessible.
Anyway,
Reviewed-by: Peter Maydell
thanks
-- PMM
ATB,
Mark.
Re: [PATCH v6 01/19] configure, meson: override C compiler for cmake
On 2/20/22 09:27, Paolo Bonzini wrote: On 2/18/22 15:49, Jag Raman wrote: Concerning the generated files, I see the following in CMakeMesonToolchainFile.cmake: Without patch: set(CMAKE_C_COMPILER "/opt/rh/devtoolset-9/root/usr/bin/cc" "-m64" "-mcx16”) With patch: set(CMAKE_C_COMPILER "cc" "-m64" "-mcx16") I don't understand why it works at all with the latter, but the right solution could be set(CMAKE_C_COMPILER "/opt/rh/devtoolset-9/root/usr/bin/cc") set(CMAKE_C_COMPILER_ARG1 "-m64") set(CMAKE_C_COMPILER_ARG2 "-mcx16") Anyhow it seems to be a cmake bug, because what QEMU/Meson are doing is exactly what is in the manual at https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER.html#variable:CMAKE_%3CLANG%3E_COMPILER: Note: Options that are required to make the compiler work correctly can be included as items in a list; they can not be changed. #set within user supplied toolchain file set(CMAKE_C_COMPILER /full/path/to/qcc --arg1 --arg2) Paolo
[PATCH] target/nios2: Shadow register set, EIC and VIC
>From 99efcd655e83f034bce25271fe592d8789529e54 Mon Sep 17 00:00:00 2001
From: Amir Gonnen
Date: Thu, 17 Feb 2022 15:43:14 +0200
Subject: [PATCH] target/nios2: Shadow register set, EIC and VIC
Implement nios2 Vectored Interrupt Controller (VIC).
This includes Exteral Interrupt Controller interface (EIC) and Shadow
Register set implementation on the nios2 cpu.
Implemented missing wrprs and rdprs instructions, and fixed eret.
Added intc_present property, true by default. When set to false, nios2
uses the EIC interface when handling IRQ.
To use VIC, the nios2 board should set VIC cpu property, disable
intc_present, connect VIC irq to cpu and connect VIC gpios.
Signed-off-by: Amir Gonnen
---
hw/intc/Kconfig | 4 +
hw/intc/meson.build | 1 +
hw/intc/nios2_vic.c | 327 +++
target/nios2/cpu.c | 59 +--
target/nios2/cpu.h | 69 -
target/nios2/helper.c| 33 +++-
target/nios2/helper.h| 3 +
target/nios2/op_helper.c | 31 +++-
target/nios2/translate.c | 32 +++-
9 files changed, 537 insertions(+), 22 deletions(-)
create mode 100644 hw/intc/nios2_vic.c
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 528e77b4a6..8000241428 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -81,3 +81,7 @@ config GOLDFISH_PIC
config M68K_IRQC
bool
+
+config NIOS2_VIC
+default y
+depends on NIOS2 && TCG
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 7466024402..547e16eb2d 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -61,3 +61,4 @@ specific_ss.add(when: ['CONFIG_KVM', 'CONFIG_XIVE'],
if_true: files('spapr_xive_kvm.c'))
specific_ss.add(when: 'CONFIG_GOLDFISH_PIC', if_true: files('goldfish_pic.c'))
specific_ss.add(when: 'CONFIG_M68K_IRQC', if_true: files('m68k_irqc.c'))
+specific_ss.add(when: 'CONFIG_NIOS2_VIC', if_true: files('nios2_vic.c'))
diff --git a/hw/intc/nios2_vic.c b/hw/intc/nios2_vic.c
new file mode 100644
index 00..a9c9b3e7ac
--- /dev/null
+++ b/hw/intc/nios2_vic.c
@@ -0,0 +1,327 @@
+/*
+ * Vectored Interrupt Controller for nios2 processor
+ *
+ * Copyright (c) 2022 Neuroblade
+ *
+ * Interface:
+ * QOM property "cpu": link to the Nios2 CPU (must be set)
+ * Unnamed GPIO inputs 0..NIOS2_VIC_MAX_IRQ-1: input IRQ lines
+ * IRQ should be connected to nios2 IRQ0.
+ *
+ * Reference: "Embedded Peripherals IP User Guide
+ * for Intel® Quartus® Prime Design Suite: 21.4"
+ * Chapter 38 "Vectored Interrupt Controller Core"
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "qom/object.h"
+#include "cpu.h"
+
+#define LOG_VIC(...) qemu_log_mask(CPU_LOG_INT, ##__VA_ARGS__)
+
+#define TYPE_NIOS2_VIC "nios2-vic"
+
+OBJECT_DECLARE_SIMPLE_TYPE(Nios2Vic, NIOS2_VIC)
+
+#define NIOS2_VIC_MAX_IRQ 32
+
+enum {
+INT_CONFIG0 = 0,
+INT_CONFIG31 = 31,
+INT_ENABLE = 32,
+INT_ENABLE_SET = 33,
+INT_ENABLE_CLR = 34,
+INT_PENDING = 35,
+INT_RAW_STATUS = 36,
+SW_INTERRUPT = 37,
+SW_INTERRUPT_SET = 38,
+SW_INTERRUPT_CLR = 39,
+VIC_CONFIG = 40,
+VIC_STATUS = 41,
+VEC_TBL_BASE = 42,
+VEC_TBL_ADDR = 43,
+CSR_COUNT /* Last! */
+};
+
+struct Nios2Vic {
+/*< private >*/
+SysBusDevice parent_obj;
+
+/*< public >*/
+qemu_irq output_int;
+
+/* properties */
+CPUState *cpu;
+MemoryRegion csr;
+
+uint32_t int_config[32];
+uint32_t vic_config;
+uint32_t int_raw_status;
+uint32_t int_enable;
+uint32_t sw_int;
+uint32_t vic_status;
+uint32_t vec_tbl_base;
+uint32_t vec_tbl_addr;
+};
+
+/* Requested interrupt level (INT_CONFIG[0:5]) */
+static inline uint32_t vic_int_config_ril(const Nios2Vic *vic, int irq_num)
+{
+
Re: [PULL 00/12] virtiofs queue
On Thu, 17 Feb 2022 at 17:30, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" > > The following changes since commit c13b8e9973635f34f3ce4356af27a311c993729c: > > Merge remote-tracking branch > 'remotes/alistair/tags/pull-riscv-to-apply-20220216' into staging (2022-02-16 > 09:57:11 +) > > are available in the Git repository at: > > https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20220217b > > for you to fetch changes up to 45b04ef48dbbeb18d93c2631bf5584ac493de749: > > virtiofsd: Add basic support for FUSE_SYNCFS request (2022-02-17 17:22:26 > +) > > > V3: virtiofs pull 2022-02-17 > > Security label improvements from Vivek > - includes a fix for building against new kernel headers > [V3: checkpatch style fixes] > [V2: Fix building on old Linux] > Blocking flock disable from Sebastian > SYNCFS support from Greg > > Signed-off-by: Dr. David Alan Gilbert > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0 for any user-visible changes. -- PMM
Re: [PATCH] target/nios2: Shadow register set, EIC and VIC
On Sun, 20 Feb 2022 at 13:07, wrote:
>
> From 99efcd655e83f034bce25271fe592d8789529e54 Mon Sep 17 00:00:00 2001
> From: Amir Gonnen
> Date: Thu, 17 Feb 2022 15:43:14 +0200
> Subject: [PATCH] target/nios2: Shadow register set, EIC and VIC
>
> Implement nios2 Vectored Interrupt Controller (VIC).
> This includes Exteral Interrupt Controller interface (EIC) and Shadow
> Register set implementation on the nios2 cpu.
> Implemented missing wrprs and rdprs instructions, and fixed eret.
> Added intc_present property, true by default. When set to false, nios2
> uses the EIC interface when handling IRQ.
Hi; this patch seems to be trying to fix multiple things
at once. Could you split it up into a multi-patch patch series,
where each patch does one logical thing, please? In particular
bug fixes to existing code ("fixed eret") should be their
own patches, separate from patches adding new features.
> To use VIC, the nios2 board should set VIC cpu property, disable
> intc_present, connect VIC irq to cpu and connect VIC gpios.
Is there a patch which wires up one of the nios2 boards to do this ?
https://www.qemu.org/docs/master/devel/submitting-a-patch.html
is our guidelines on patch formatting.
thanks
-- PMM
Re: configure: How to pass flags to the Objective-C compiler?
On 2/19/22 20:17, Joshua Seaton wrote: Is that accurate? If so, will there be follow-up patches? If not, could you clarify how this amounts to affecting Objective-C compilation steps? This entry in the machine file affects the compilation steps: + test -n "$objcc" && echo "objc_args = [$(meson_quote $OBJCFLAGS $EXTRA_OBJCFLAGS)]" >> $cross Paolo
[PATCH v8 05/11] 9p: darwin: Ignore O_{NOATIME, DIRECT}
From: Keno Fischer
Darwin doesn't have either of these flags. Darwin does have
F_NOCACHE, which is similar to O_DIRECT, but has different
enough semantics that other projects don't generally map
them automatically. In any case, we don't support O_DIRECT
on Linux at the moment either.
Signed-off-by: Keno Fischer
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch
[Will Cohen: - Adjust coding style]
Signed-off-by: Will Cohen
---
hw/9pfs/9p-util.h | 2 ++
hw/9pfs/9p.c | 13 -
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index d41f37f085..0e445b5d52 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -41,6 +41,7 @@ again:
fd = openat(dirfd, name, flags | O_NOFOLLOW | O_NOCTTY | O_NONBLOCK,
mode);
if (fd == -1) {
+#ifndef CONFIG_DARWIN
if (errno == EPERM && (flags & O_NOATIME)) {
/*
* The client passed O_NOATIME but we lack permissions to honor it.
@@ -53,6 +54,7 @@ again:
flags &= ~O_NOATIME;
goto again;
}
+#endif
return -1;
}
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index caf3b240fe..14e84c3bcf 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -138,11 +138,20 @@ static int dotl_to_open_flags(int flags)
{ P9_DOTL_NONBLOCK, O_NONBLOCK } ,
{ P9_DOTL_DSYNC, O_DSYNC },
{ P9_DOTL_FASYNC, FASYNC },
+#ifndef CONFIG_DARWIN
+{ P9_DOTL_NOATIME, O_NOATIME },
+/*
+ * On Darwin, we could map to F_NOCACHE, which is
+ * similar, but doesn't quite have the same
+ * semantics. However, we don't support O_DIRECT
+ * even on linux at the moment, so we just ignore
+ * it here.
+ */
{ P9_DOTL_DIRECT, O_DIRECT },
+#endif
{ P9_DOTL_LARGEFILE, O_LARGEFILE },
{ P9_DOTL_DIRECTORY, O_DIRECTORY },
{ P9_DOTL_NOFOLLOW, O_NOFOLLOW },
-{ P9_DOTL_NOATIME, O_NOATIME },
{ P9_DOTL_SYNC, O_SYNC },
};
@@ -171,10 +180,12 @@ static int get_dotl_openflags(V9fsState *s, int oflags)
*/
flags = dotl_to_open_flags(oflags);
flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT);
+#ifndef CONFIG_DARWIN
/*
* Ignore direct disk access hint until the server supports it.
*/
flags &= ~O_DIRECT;
+#endif
return flags;
}
--
2.35.1
[PATCH v8 00/11] 9p: Add support for darwin
This is a followup to
https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg04298.html,
adding 9p server support for Darwin.
Since v7, no functional changes have been made to this patch set, but Patch 9/11
(9p: darwin: Implement compatibility for mknodat) was rebased to apply cleanly
on top
of the most recent changes to 9pfs, which affected the code changes osdep.h
directly
above patch 9’s additions.
WIth these changes, v8 correctly applies and functions on the latest mainline
qemu.
Keno Fischer (10):
9p: linux: Fix a couple Linux assumptions
9p: Rename 9p-util -> 9p-util-linux
9p: darwin: Handle struct stat(fs) differences
9p: darwin: Handle struct dirent differences
9p: darwin: Ignore O_{NOATIME, DIRECT}
9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX
9p: darwin: *xattr_nofollow implementations
9p: darwin: Compatibility for f/l*xattr
9p: darwin: Implement compatibility for mknodat
9p: darwin: meson: Allow VirtFS on Darwin
Will Cohen (1):
9p: darwin: Adjust assumption on virtio-9p-test
fsdev/file-op-9p.h | 9 +++-
fsdev/meson.build | 1 +
hw/9pfs/9p-local.c | 27 ---
hw/9pfs/9p-proxy.c | 38 +--
hw/9pfs/9p-synth.c | 6 +++
hw/9pfs/9p-util-darwin.c | 64 ++
hw/9pfs/{9p-util.c => 9p-util-linux.c} | 2 +-
hw/9pfs/9p-util.h | 35 ++
hw/9pfs/9p.c | 42 ++---
hw/9pfs/9p.h | 18
hw/9pfs/codir.c| 4 +-
hw/9pfs/meson.build| 3 +-
include/qemu/osdep.h | 12 +
include/qemu/xattr.h | 4 +-
meson.build| 15 --
os-posix.c | 35 ++
tests/qtest/virtio-9p-test.c | 2 +-
17 files changed, 292 insertions(+), 25 deletions(-)
create mode 100644 hw/9pfs/9p-util-darwin.c
rename hw/9pfs/{9p-util.c => 9p-util-linux.c} (97%)
--
2.35.1
[PATCH v8 02/11] 9p: Rename 9p-util -> 9p-util-linux
From: Keno Fischer
The current file only has the Linux versions of these functions.
Rename the file accordingly and update the Makefile to only build
it on Linux. A Darwin version of these will follow later in the
series.
Signed-off-by: Keno Fischer
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch
Signed-off-by: Will Cohen
Reviewed-by: Greg Kurz
Reviewed-by: Philippe Mathieu-Daudé
---
hw/9pfs/{9p-util.c => 9p-util-linux.c} | 2 +-
hw/9pfs/meson.build| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
rename hw/9pfs/{9p-util.c => 9p-util-linux.c} (97%)
diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util-linux.c
similarity index 97%
rename from hw/9pfs/9p-util.c
rename to hw/9pfs/9p-util-linux.c
index 3221d9b498..398614a5d0 100644
--- a/hw/9pfs/9p-util.c
+++ b/hw/9pfs/9p-util-linux.c
@@ -1,5 +1,5 @@
/*
- * 9p utilities
+ * 9p utilities (Linux Implementation)
*
* Copyright IBM, Corp. 2017
*
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 99be5d9119..1b28e70040 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -4,7 +4,6 @@ fs_ss.add(files(
'9p-posix-acl.c',
'9p-proxy.c',
'9p-synth.c',
- '9p-util.c',
'9p-xattr-user.c',
'9p-xattr.c',
'9p.c',
@@ -14,6 +13,7 @@ fs_ss.add(files(
'coth.c',
'coxattr.c',
))
+fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c'))
fs_ss.add(when: 'CONFIG_XEN', if_true: files('xen-9p-backend.c'))
softmmu_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
--
2.35.1
[PATCH v8 03/11] 9p: darwin: Handle struct stat(fs) differences
From: Keno Fischer Signed-off-by: Keno Fischer Signed-off-by: Michael Roitzsch [Will Cohen: - Note lack of f_namelen and f_frsize on Darwin - Ensure that tv_sec and tv_nsec are both initialized for Darwin and non-Darwin] Signed-off-by: Will Cohen --- hw/9pfs/9p-proxy.c | 22 -- hw/9pfs/9p-synth.c | 2 ++ hw/9pfs/9p.c | 16 ++-- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c index 09bd9f1464..b1664080d8 100644 --- a/hw/9pfs/9p-proxy.c +++ b/hw/9pfs/9p-proxy.c @@ -123,10 +123,16 @@ static void prstatfs_to_statfs(struct statfs *stfs, ProxyStatFS *prstfs) stfs->f_bavail = prstfs->f_bavail; stfs->f_files = prstfs->f_files; stfs->f_ffree = prstfs->f_ffree; +#ifdef CONFIG_DARWIN +/* f_namelen and f_frsize do not exist on Darwin */ +stfs->f_fsid.val[0] = prstfs->f_fsid[0] & 0xU; +stfs->f_fsid.val[1] = prstfs->f_fsid[1] >> 32 & 0xU; +#else stfs->f_fsid.__val[0] = prstfs->f_fsid[0] & 0xU; stfs->f_fsid.__val[1] = prstfs->f_fsid[1] >> 32 & 0xU; stfs->f_namelen = prstfs->f_namelen; stfs->f_frsize = prstfs->f_frsize; +#endif } /* Converts proxy_stat structure to VFS stat structure */ @@ -143,12 +149,24 @@ static void prstat_to_stat(struct stat *stbuf, ProxyStat *prstat) stbuf->st_size = prstat->st_size; stbuf->st_blksize = prstat->st_blksize; stbuf->st_blocks = prstat->st_blocks; + stbuf->st_atime = prstat->st_atim_sec; + stbuf->st_mtime = prstat->st_mtim_sec; + stbuf->st_ctime = prstat->st_ctim_sec; +#ifdef CONFIG_DARWIN + stbuf->st_atimespec.tv_sec = prstat->st_atim_sec; + stbuf->st_mtimespec.tv_sec = prstat->st_mtim_sec; + stbuf->st_ctimespec.tv_sec = prstat->st_ctim_sec; + stbuf->st_atimespec.tv_nsec = prstat->st_atim_nsec; + stbuf->st_mtimespec.tv_nsec = prstat->st_mtim_nsec; + stbuf->st_ctimespec.tv_nsec = prstat->st_ctim_nsec; +#else stbuf->st_atim.tv_sec = prstat->st_atim_sec; + stbuf->st_mtim.tv_sec = prstat->st_mtim_sec; + stbuf->st_ctim.tv_sec = prstat->st_ctim_sec; stbuf->st_atim.tv_nsec = prstat->st_atim_nsec; - stbuf->st_mtime = prstat->st_mtim_sec; stbuf->st_mtim.tv_nsec = prstat->st_mtim_nsec; - stbuf->st_ctime = prstat->st_ctim_sec; stbuf->st_ctim.tv_nsec = prstat->st_ctim_nsec; +#endif } /* diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c index 7a7cd5c5ba..bf9b0c5ddd 100644 --- a/hw/9pfs/9p-synth.c +++ b/hw/9pfs/9p-synth.c @@ -439,7 +439,9 @@ static int synth_statfs(FsContext *s, V9fsPath *fs_path, stbuf->f_bsize = 512; stbuf->f_blocks = 0; stbuf->f_files = synth_node_count; +#ifndef CONFIG_DARWIN stbuf->f_namelen = NAME_MAX; +#endif return 0; } diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 9c63e14b28..1563d7b7c6 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1313,11 +1313,17 @@ static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf, v9lstat->st_blksize = stat_to_iounit(pdu, stbuf); v9lstat->st_blocks = stbuf->st_blocks; v9lstat->st_atime_sec = stbuf->st_atime; -v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec; v9lstat->st_mtime_sec = stbuf->st_mtime; -v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec; v9lstat->st_ctime_sec = stbuf->st_ctime; +#ifdef CONFIG_DARWIN +v9lstat->st_atime_nsec = stbuf->st_atimespec.tv_nsec; +v9lstat->st_mtime_nsec = stbuf->st_mtimespec.tv_nsec; +v9lstat->st_ctime_nsec = stbuf->st_ctimespec.tv_nsec; +#else +v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec; +v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec; v9lstat->st_ctime_nsec = stbuf->st_ctim.tv_nsec; +#endif /* Currently we only support BASIC fields in stat */ v9lstat->st_result_mask = P9_STATS_BASIC; @@ -3519,9 +3525,15 @@ static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf) f_bavail = stbuf->f_bavail / bsize_factor; f_files = stbuf->f_files; f_ffree = stbuf->f_ffree; +#ifdef CONFIG_DARWIN +fsid_val = (unsigned int)stbuf->f_fsid.val[0] | + (unsigned long long)stbuf->f_fsid.val[1] << 32; +f_namelen = NAME_MAX; +#else fsid_val = (unsigned int) stbuf->f_fsid.__val[0] | (unsigned long long)stbuf->f_fsid.__val[1] << 32; f_namelen = stbuf->f_namelen; +#endif return pdu_marshal(pdu, offset, "ddqqd", f_type, f_bsize, f_blocks, f_bfree, -- 2.35.1
[PATCH v8 06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX
From: Keno Fischer
Signed-off-by: Keno Fischer
Signed-off-by: Michael Roitzsch
Because XATTR_SIZE_MAX is not defined on Darwin,
create a cross-platform P9_XATTR_SIZE_MAX instead.
[Will Cohen: - Adjust coding style
- Lower XATTR_SIZE_MAX to 64k
- Add explanatory context related to XATTR_SIZE_MAX]
[Fabian Franz: - Move XATTR_SIZE_MAX reference from 9p.c to
P9_XATTR_SIZE_MAX in 9p.h]
Signed-off-by: Will Cohen
Signed-off-by: Fabian Franz
[Will Cohen: - For P9_XATTR_MAX, ensure that Linux uses
XATTR_SIZE_MAX, Darwin uses 64k, and error
out for undefined hosts]
Signed-off-by: Will Cohen
---
hw/9pfs/9p.c | 2 +-
hw/9pfs/9p.h | 18 ++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 14e84c3bcf..7405352c37 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3949,7 +3949,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
rflags |= XATTR_REPLACE;
}
-if (size > XATTR_SIZE_MAX) {
+if (size > P9_XATTR_SIZE_MAX) {
err = -E2BIG;
goto out_nofid;
}
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 1567b67841..94b273b3d0 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -479,4 +479,22 @@ struct V9fsTransport {
void(*push_and_notify)(V9fsPDU *pdu);
};
+#if defined(XATTR_SIZE_MAX)
+/* Linux */
+#define P9_XATTR_SIZE_MAX XATTR_SIZE_MAX
+#elif defined(CONFIG_DARWIN)
+/*
+ * Darwin doesn't seem to define a maximum xattr size in its user
+ * space header, so manually configure it across platforms as 64k.
+ *
+ * Having no limit at all can lead to QEMU crashing during large g_malloc()
+ * calls. Because QEMU does not currently support macOS guests, the below
+ * preliminary solution only works due to its being a reflection of the limit
of
+ * Linux guests.
+ */
+#define P9_XATTR_SIZE_MAX 65536
+#else
+#error Missing definition for P9_XATTR_SIZE_MAX for this host system
+#endif
+
#endif
--
2.35.1
[PATCH v8 01/11] 9p: linux: Fix a couple Linux assumptions
From: Keno Fischer - Guard Linux only headers. - Add qemu/statfs.h header to abstract over the which headers are needed for struct statfs - Define `ENOATTR` only if not only defined (it's defined in system headers on Darwin). Signed-off-by: Keno Fischer [Michael Roitzsch: - Rebase for NixOS] Signed-off-by: Michael Roitzsch While it might at first appear that fsdev/virtfs-proxy-header.c would need similar adjustment for darwin as file-op-9p here, a later patch in this series disables virtfs-proxy-helper for non-Linux. Allowing virtfs-proxy-helper on darwin could potentially be an additional optimization later. [Will Cohen: - Fix headers for Alpine - Integrate statfs.h back into file-op-9p.h - Remove superfluous header guards from file-opt-9p - Add note about virtfs-proxy-helper being disabled on non-Linux for this patch series] Signed-off-by: Will Cohen Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Greg Kurz --- fsdev/file-op-9p.h | 9 - hw/9pfs/9p-local.c | 2 ++ hw/9pfs/9p.c | 4 include/qemu/xattr.h | 4 +++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h index 8fd89f0447..4997677460 100644 --- a/fsdev/file-op-9p.h +++ b/fsdev/file-op-9p.h @@ -16,10 +16,17 @@ #include #include -#include #include "qemu-fsdev-throttle.h" #include "p9array.h" +#ifdef CONFIG_LINUX +# include +#endif +#ifdef CONFIG_DARWIN +# include +# include +#endif + #define SM_LOCAL_MODE_BITS0600 #define SM_LOCAL_DIR_MODE_BITS0700 diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index 210d9e7705..1a5e3eed73 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -32,10 +32,12 @@ #include "qemu/error-report.h" #include "qemu/option.h" #include +#ifdef CONFIG_LINUX #include #ifdef CONFIG_LINUX_MAGIC_H #include #endif +#endif #include #ifndef XFS_SUPER_MAGIC diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 15b3f4d385..9c63e14b28 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -32,7 +32,11 @@ #include "migration/blocker.h" #include "qemu/xxhash.h" #include +#ifdef CONFIG_LINUX #include +#else +#include +#endif int open_fd_hw; int total_open_fd; diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h index a83fe8e749..f1d0f7be74 100644 --- a/include/qemu/xattr.h +++ b/include/qemu/xattr.h @@ -22,7 +22,9 @@ #ifdef CONFIG_LIBATTR # include #else -# define ENOATTR ENODATA +# if !defined(ENOATTR) +#define ENOATTR ENODATA +# endif # include #endif -- 2.35.1
[PATCH v8 04/11] 9p: darwin: Handle struct dirent differences
From: Keno Fischer
On darwin d_seekoff exists, but is optional and does not seem to
be commonly used by file systems. Use `telldir` instead to obtain
the seek offset and inject it into d_seekoff, and create a
qemu_dirent_off helper to call it appropriately when appropriate.
Signed-off-by: Keno Fischer
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch
[Will Cohen: - Adjust to pass testing
- Ensure that d_seekoff is filled using telldir
on darwin, and create qemu_dirent_off helper
to decide which to access]
[Fabian Franz: - Add telldir error handling for darwin]
Signed-off-by: Fabian Franz
[Will Cohen: - Ensure that telldir error handling uses
signed int
- Cleanup of telldir error handling
- Remove superfluous error handling for
qemu_dirent_off
- Adjust formatting
- Use qemu_dirent_off in codir.c]
Signed-off-by: Will Cohen
---
hw/9pfs/9p-local.c | 9 +
hw/9pfs/9p-proxy.c | 16 +++-
hw/9pfs/9p-synth.c | 4
hw/9pfs/9p-util.h | 16
hw/9pfs/9p.c | 7 +--
hw/9pfs/codir.c| 4 +++-
6 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 1a5e3eed73..f3272f0b43 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -562,6 +562,15 @@ again:
if (!entry) {
return NULL;
}
+#ifdef CONFIG_DARWIN
+int off;
+off = telldir(fs->dir.stream);
+/* If telldir fails, fail the entire readdir call */
+if (off < 0) {
+return NULL;
+}
+entry->d_seekoff = off;
+#endif
if (ctx->export_flags & V9FS_SM_MAPPED) {
entry->d_type = DT_UNKNOWN;
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index b1664080d8..8b4b5cf7dc 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx,
V9fsFidOpenState *fs)
static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState *fs)
{
-return readdir(fs->dir.stream);
+struct dirent *entry;
+entry = readdir(fs->dir.stream);
+#ifdef CONFIG_DARWIN
+if (!entry) {
+return NULL;
+}
+int td;
+td = telldir(fs->dir.stream);
+/* If telldir fails, fail the entire readdir call */
+if (td < 0) {
+return NULL;
+}
+entry->d_seekoff = td;
+#endif
+return entry;
}
static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off)
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index bf9b0c5ddd..b3080e415b 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node,
offsetof(struct dirent, d_name) + sz);
memcpy(entry->d_name, node->name, sz);
entry->d_ino = node->attr->inode;
+#ifdef CONFIG_DARWIN
+entry->d_seekoff = off + 1;
+#else
entry->d_off = off + 1;
+#endif
}
static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 546f46dc7d..d41f37f085 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -79,3 +79,19 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char
*filename,
const char *name);
#endif
+
+
+/**
+ * Darwin has d_seekoff, which appears to function similarly to d_off.
+ * However, it does not appear to be supported on all file systems,
+ * so ensure it is manually injected earlier and call here when
+ * needed.
+ */
+inline off_t qemu_dirent_off(struct dirent *dent)
+{
+#ifdef CONFIG_DARWIN
+return dent->d_seekoff;
+#else
+return dent->d_off;
+#endif
+}
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 1563d7b7c6..caf3b240fe 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -27,6 +27,7 @@
#include "virtio-9p.h"
#include "fsdev/qemu-fsdev.h"
#include "9p-xattr.h"
+#include "9p-util.h"
#include "coth.h"
#include "trace.h"
#include "migration/blocker.h"
@@ -2281,7 +2282,7 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU
*pdu,
count += len;
v9fs_stat_free(&v9stat);
v9fs_path_free(&path);
-saved_dir_pos = dent->d_off;
+saved_dir_pos = qemu_dirent_off(dent);
}
v9fs_readdir_unlock(&fidp->fs.dir);
@@ -2420,6 +2421,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu,
V9fsFidState *fidp,
V9fsString name;
int len, err = 0;
int32_t count = 0;
+off_t off;
struct dirent *dent;
struct stat *st;
struct V9fsDirEnt *entries = NULL;
@@ -2480,12 +2482,13 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu,
V9fsFidState *fidp,
qid.version = 0;
}
+off = qemu_dirent_off(dent);
v9fs_string_init(&name);
v9fs_string_sprintf(&name, "%s", dent->d_name);
/* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
len = pdu_marshal(pdu, 11 + count, "Qqbs",
-
[PATCH v8 09/11] 9p: darwin: Implement compatibility for mknodat
From: Keno Fischer
Darwin does not support mknodat. However, to avoid race conditions
with later setting the permissions, we must avoid using mknod on
the full path instead. We could try to fchdir, but that would cause
problems if multiple threads try to call mknodat at the same time.
However, luckily there is a solution: Darwin includes a function
that sets the cwd for the current thread only.
This should suffice to use mknod safely.
This function (pthread_fchdir_np) is protected by a check in
meson in a patch later in this series.
Signed-off-by: Keno Fischer
Signed-off-by: Michael Roitzsch
[Will Cohen: - Adjust coding style
- Replace clang references with gcc
- Note radar filed with Apple for missing syscall
- Replace direct syscall with pthread_fchdir_np and
adjust patch notes accordingly
- Move qemu_mknodat from 9p-util to osdep and os-posix
- Move pthread_fchdir_np declaration only to osdep
- Declare pthread_fchdir_np with
- __attribute__((weak_import)) to allow checking for
its presence before usage
- Move declarations above cplusplus guard
- Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
presence in osdep.h and os-posix.c
- Rebase to apply cleanly on top of the 2022-02-10
changes to 9pfs]
Signed-off-by: Will Cohen
---
hw/9pfs/9p-local.c | 4 ++--
include/qemu/osdep.h | 12
meson.build | 1 +
os-posix.c | 35 +++
4 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index a0d08e5216..d42ce6d8b8 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
*dir_path,
if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
-err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
+err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
if (err == -1) {
goto out;
}
@@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
*dir_path,
}
} else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
fs_ctx->export_flags & V9FS_SM_NONE) {
-err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
+err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
if (err == -1) {
goto out;
}
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ce12f64853..c0f442d791 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -818,6 +818,18 @@ static inline int platform_does_not_support_system(const
char *command)
*/
struct dirent *qemu_dirent_dup(struct dirent *dent);
+/*
+ * As long as mknodat is not available on macOS, this workaround
+ * using pthread_fchdir_np is needed. qemu_mknodat is defined in
+ * os-posix.c. pthread_fchdir_np is weakly linked here as a guard
+ * in case it disappears in future macOS versions, because it is
+ * is a private API.
+ */
+#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
+int pthread_fchdir_np(int fd) __attribute__((weak_import));
+#endif
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev);
+
#ifdef __cplusplus
}
#endif
diff --git a/meson.build b/meson.build
index ae5f7eec6e..6fdc0281ad 100644
--- a/meson.build
+++ b/meson.build
@@ -1557,6 +1557,7 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE',
cc.has_function('posix_fallocate'
config_host_data.set('CONFIG_POSIX_MEMALIGN',
cc.has_function('posix_memalign'))
config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
'#include '))
+config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
cc.has_function('pthread_fchdir_np'))
config_host_data.set('CONFIG_SEM_TIMEDWAIT', cc.has_function('sem_timedwait',
dependencies: threads))
config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
cc.has_function('unshare'))
diff --git a/os-posix.c b/os-posix.c
index ae6c9f2a5e..ccc3d1e9d3 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -332,3 +332,38 @@ int os_mlock(void)
return -ENOSYS;
#endif
}
+
+/*
+ * As long as mknodat is not available on macOS, this workaround
+ * using pthread_fchdir_np is needed.
+ *
+ * Radar filed with Apple for implementing mknodat:
+ * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
+ */
+#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
+
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
+{
+int preserved_errno, err;
+if (!pthread_fchdir_np) {
+error_report_once("pthread_fchdir_np() is not available on this
version of macOS");
+return -ENOTSUP;
+}
+
[PATCH v8 08/11] 9p: darwin: Compatibility for f/l*xattr
From: Keno Fischer
On darwin `fgetxattr` takes two extra optional arguments,
and the l* variants are not defined (in favor of an extra
flag to the regular variants.
Signed-off-by: Keno Fischer
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch
Signed-off-by: Will Cohen
---
hw/9pfs/9p-local.c | 12
hw/9pfs/9p-util.h | 17 +
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index f3272f0b43..a0d08e5216 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -790,16 +790,20 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
mode_t tmp_mode;
dev_t tmp_dev;
-if (fgetxattr(fd, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.uid",
+ &tmp_uid, sizeof(uid_t)) > 0) {
stbuf->st_uid = le32_to_cpu(tmp_uid);
}
-if (fgetxattr(fd, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.gid",
+ &tmp_gid, sizeof(gid_t)) > 0) {
stbuf->st_gid = le32_to_cpu(tmp_gid);
}
-if (fgetxattr(fd, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.mode",
+ &tmp_mode, sizeof(mode_t)) > 0) {
stbuf->st_mode = le32_to_cpu(tmp_mode);
}
-if (fgetxattr(fd, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.rdev",
+ &tmp_dev, sizeof(dev_t)) > 0) {
stbuf->st_rdev = le64_to_cpu(tmp_dev);
}
} else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 0e445b5d52..82399702b9 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -19,6 +19,23 @@
#define O_PATH_9P_UTIL 0
#endif
+#ifdef CONFIG_DARWIN
+#define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
+#define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
+#define qemu_llistxattr(...) listxattr(__VA_ARGS__, XATTR_NOFOLLOW)
+#define qemu_lremovexattr(...) removexattr(__VA_ARGS__, XATTR_NOFOLLOW)
+static inline int qemu_lsetxattr(const char *path, const char *name,
+ const void *value, size_t size, int flags) {
+return setxattr(path, name, value, size, 0, flags | XATTR_NOFOLLOW);
+}
+#else
+#define qemu_fgetxattr fgetxattr
+#define qemu_lgetxattr lgetxattr
+#define qemu_llistxattr llistxattr
+#define qemu_lremovexattr lremovexattr
+#define qemu_lsetxattr lsetxattr
+#endif
+
static inline void close_preserve_errno(int fd)
{
int serrno = errno;
--
2.35.1
[PATCH v8 07/11] 9p: darwin: *xattr_nofollow implementations
From: Keno Fischer
This implements the darwin equivalent of the functions that were
moved to 9p-util(-linux) earlier in this series in the new
9p-util-darwin file.
Signed-off-by: Keno Fischer
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch
Signed-off-by: Will Cohen
---
hw/9pfs/9p-util-darwin.c | 64
hw/9pfs/meson.build | 1 +
2 files changed, 65 insertions(+)
create mode 100644 hw/9pfs/9p-util-darwin.c
diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
new file mode 100644
index 00..cdb4c9e24c
--- /dev/null
+++ b/hw/9pfs/9p-util-darwin.c
@@ -0,0 +1,64 @@
+/*
+ * 9p utilities (Darwin Implementation)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/xattr.h"
+#include "9p-util.h"
+
+ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+ void *value, size_t size)
+{
+int ret;
+int fd = openat_file(dirfd, filename,
+ O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = fgetxattr(fd, name, value, size, 0, 0);
+close_preserve_errno(fd);
+return ret;
+}
+
+ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
+ char *list, size_t size)
+{
+int ret;
+int fd = openat_file(dirfd, filename,
+ O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = flistxattr(fd, list, size, 0);
+close_preserve_errno(fd);
+return ret;
+}
+
+ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
+const char *name)
+{
+int ret;
+int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = fremovexattr(fd, name, 0);
+close_preserve_errno(fd);
+return ret;
+}
+
+int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+ void *value, size_t size, int flags)
+{
+int ret;
+int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = fsetxattr(fd, name, value, size, 0, flags);
+close_preserve_errno(fd);
+return ret;
+}
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 1b28e70040..12443b6ad5 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -14,6 +14,7 @@ fs_ss.add(files(
'coxattr.c',
))
fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c'))
+fs_ss.add(when: 'CONFIG_DARWIN', if_true: files('9p-util-darwin.c'))
fs_ss.add(when: 'CONFIG_XEN', if_true: files('xen-9p-backend.c'))
softmmu_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
--
2.35.1
[PATCH v8 10/11] 9p: darwin: Adjust assumption on virtio-9p-test
The previous test depended on the assumption that P9_DOTL_AT_REMOVEDIR and AT_REMOVEDIR have the same value. While this is true on Linux, it is not true everywhere, and leads to an incorrect test failure on unlink_at, noticed when adding 9p to darwin: Received response 7 (RLERROR) instead of 77 (RUNLINKAT) Rlerror has errno 22 (Invalid argument) ** ERROR:../tests/qtest/virtio-9p-test.c:305:v9fs_req_recv: assertion failed (hdr.id == id): (7 == 77) Bail out! ERROR:../tests/qtest/virtio-9p-test.c:305:v9fs_req_recv: assertion failed (hdr.id == id): (7 == 77) Signed-off-by: Fabian Franz [Will Cohen: - Add explanation of patch and description of pre-patch test failure] Signed-off-by: Will Cohen Acked-by: Thomas Huth [Will Cohen: - Move this patch before 9p: darwin: meson patch to avoid qtest breakage during bisecting] Signed-off-by: Will Cohen Reviewed-by: Greg Kurz --- tests/qtest/virtio-9p-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 502e5ad0c7..01ca076afe 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -1253,7 +1253,7 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) /* ... and is actually a directory */ g_assert((st.st_mode & S_IFMT) == S_IFDIR); -do_unlinkat(v9p, "/", "02", AT_REMOVEDIR); +do_unlinkat(v9p, "/", "02", P9_DOTL_AT_REMOVEDIR); /* directory should be gone now */ g_assert(stat(new_dir, &st) != 0); } -- 2.35.1
[PATCH v8 11/11] 9p: darwin: meson: Allow VirtFS on Darwin
From: Keno Fischer
To allow VirtFS on darwin, we need to check that pthread_fchdir_np is
available, which has only been available since macOS 10.12.
Additionally, virtfs_proxy_helper is disabled on Darwin. This patch
series does not currently provide an implementation of the proxy-helper,
but this functionality could be implemented later on.
Signed-off-by: Keno Fischer
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch
[Will Cohen: - Rebase to master]
Signed-off-by: Will Cohen
Reviewed-by: Paolo Bonzini
[Will Cohen: - Add check for pthread_fchdir_np to virtfs
- Add comments to patch commit
- Note that virtfs_proxy_helper does not work
on macOS
- Fully adjust meson virtfs error note to specify
macOS]
Signed-off-by: Will Cohen
---
fsdev/meson.build | 1 +
meson.build | 14 ++
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/fsdev/meson.build b/fsdev/meson.build
index adf57cc43e..b632b66348 100644
--- a/fsdev/meson.build
+++ b/fsdev/meson.build
@@ -7,6 +7,7 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
'qemu-fsdev.c',
), if_false: files('qemu-fsdev-dummy.c'))
softmmu_ss.add_all(when: 'CONFIG_LINUX', if_true: fsdev_ss)
+softmmu_ss.add_all(when: 'CONFIG_DARWIN', if_true: fsdev_ss)
if have_virtfs_proxy_helper
executable('virtfs-proxy-helper',
diff --git a/meson.build b/meson.build
index 6fdc0281ad..b6f5c57487 100644
--- a/meson.build
+++ b/meson.build
@@ -1416,17 +1416,23 @@ if not get_option('dbus_display').disabled()
endif
endif
-have_virtfs = (targetos == 'linux' and
+if targetos == 'darwin' and cc.has_function('pthread_fchdir_np')
+ have_virtfs = have_system
+else
+ have_virtfs = (targetos == 'linux' and
have_system and
libattr.found() and
libcap_ng.found())
+endif
-have_virtfs_proxy_helper = have_virtfs and have_tools
+have_virtfs_proxy_helper = targetos != 'darwin' and have_virtfs and have_tools
if get_option('virtfs').enabled()
if not have_virtfs
-if targetos != 'linux'
- error('virtio-9p (virtfs) requires Linux')
+if targetos != 'linux' and targetos != 'darwin'
+ error('virtio-9p (virtfs) requires Linux or macOS')
+elif targetos == 'darwin' and not cc.has_function('pthread_fchdir_np')
+ error('virtio-9p (virtfs) on macOS requires the presence of
pthread_fchdir_np')
elif not libcap_ng.found() or not libattr.found()
error('virtio-9p (virtfs) requires libcap-ng-devel and libattr-devel')
elif not have_system
--
2.35.1
Re: [PATCH 08/11] mos6522: add "info via" HMP command for debugging
On 08/02/2022 13:10, Daniel P. Berrangé wrote:
On Tue, Feb 08, 2022 at 01:06:59PM +, Mark Cave-Ayland wrote:
On 08/02/2022 12:49, Daniel P. Berrangé wrote:
I was under the impression that monitor_register_hmp_info_hrt() does all the
magic here i.e. it declares the underlying QMP command with an x- prefix and
effectively encapsulates the text field in a way that says "this is an
unreliable text opaque for humans"?
The monitor_register_hmp_info_hrt only does the HMP glue side, and
that's only needed if you must dynamically register the HMP command.
For statically registered commands set '.cmd_info_hrt' directly in
the hml-commands-info.hx for the HMP side.
If a qapi/ schema is needed could you explain what it should look like for
this example and where it should go? Looking at the existing .json files I
can't immediately see one which is the right place for this to live.
Take a look in qapi/machine.json for anyof the 'x-query-' commands
there. The QAPI bit is fairly simple.
if you want to see an illustration of what's different from a previous
pure HMP impl, look at:
commit dd98234c059e6bdb05a52998270df6d3d990332e
Author: Daniel P. Berrangé
Date: Wed Sep 8 10:35:43 2021 +0100
qapi: introduce x-query-roms QMP command
I see, thanks for the reference. So qapi/machine.json would be the right
place to declare the QMP part even for a specific device?
Even this approach still wouldn't work in its current form though, since as
mentioned in my previous email it seems that only the target CONFIG_*
defines and not the device CONFIG_* defines are present when processing
hmp-commands-info.hx.
Yeah, that's where the pain comes in. While QAPI schema can be made
conditional on a few CONFIG_* parameters - basically those derived
from global configure time options, it is impossible for this to be
with with target specific options like the device CONFIG_* defines.
This is why I suggested in my othuer reply that it would need to be
done with a generic 'info dev-debug' / 'x-query-dev-debug' command
that can be registered unconditionally, and then individual devices
plug into it.
After some more experiments this afternoon I still seem to be falling through the
gaps on this one. This is based upon my understanding that all new HMP commands
should use a QMP HumanReadableText implementation and the new command should be
restricted according to target.
Currently I am working with this change to hmp-commands-info.hx and
qapi/misc-target.json:
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 7e6bd30395..aac86d5473 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -880,15 +880,15 @@ SRST
Show intel SGX information.
ERST
#if defined(TARGET_M68K) || defined(TARGET_PPC)
{
.name = "via",
.args_type= "",
.params = "",
.help = "show guest mos6522 VIA devices",
.cmd_info_hrt = qmp_x_query_via,
},
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 4bc45d2474..72bf71888e 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -2,6 +2,8 @@
# vim: filetype=python
#
+{ 'include': 'common.json' }
+
##
# @RTC_CHANGE:
#
@@ -424,3 +426,19 @@
#
##
{ 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if':
'TARGET_I386' }
+
+##
+# @x-query-via:
+#
+# Query information on the registered mos6522 VIAs
+#
+# Features:
+# @unstable: This command is meant for debugging.
+#
+# Returns: internal mos6522 information
+#
+# Since: 7.0
+##
+{ 'command': 'x-query-via',
+ 'returns': 'HumanReadableText',
+ 'features': [ 'unstable' ], 'if': { 'any': [ 'TARGET_M68K', 'TARGET_PPC' ] }
}
The main problem with trying to restrict the new command to a target (or targets) is
that the autogenerated qapi/qapi-commands-misc-target.h QAPI header cannot be
included in a device header such as mos6522.h without getting poison errors like
below (which does actually make sense):
In file included from ./qapi/qapi-commands-misc-target.h:17,
from
/home/build/src/qemu/git/qemu/include/hw/misc/mos6522.h:35,
from ../hw/misc/mos6522.c:30:
./qapi/qapi-types-misc-target.h:19:13: error: attempt to use poisoned
"TARGET_ALPHA"
I can work around that by declaring the prototype for qmp_x_query_via()
manually i.e.:
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index 9c21da2ddd..9677293ad0 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -30,7 +30,7 @@
#include "exec/memory.h"
#include "hw/sysbus.h"
#include "hw/input/adb.h"
+#include "qapi/qapi-commands-common.h"
#include "qom/object.h"
/* Bits in ACR */
@@ -156,4 +156,6 @@ extern const VMStateDescription vmstate_mos6522;
uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size);
void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size);
+HumanReadableText *qmp_x_query_via(Error **errp);
+
#endif /* MOS652
RE: [PATCH] target/nios2: Shadow register set, EIC and VIC
Hi Peter,
I can split the VIC from the core+EIC changes, although the core+EIC changes
make little sense without a VIC to interact with them.
However, I'm not sure how to split the changes to the nios2 core into multiple
patches.
The shadow register set, together with the EIC interface and interrupt handling
are very much tied together.
Regarding the "fixed eret" - perhaps I didn't phrase it right. What I meant is
that eret was changed to work correctly in the presence of a shadow register
set.
So, the changes to eret are part of the shadow register set support on the core
and cannot exist on their own.
I tested this on Neuroblade board with JUART. I did not wire it to an existing
demo board.
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html
> is our guidelines on patch formatting.
In fact, I tried to follow them. I've also run checkpatch.pl, etc.
Could you please point out where I failed to follow them or what I'm missing?
Thanks,
Amir
-Original Message-
From: Peter Maydell
Sent: Sunday, February 20, 2022 5:13 PM
To: Amir Gonnen
Cc: qemu-devel@nongnu.org; Chris Wulff ; Marek Vasut
Subject: Re: [PATCH] target/nios2: Shadow register set, EIC and VIC
[EXTERNAL]
On Sun, 20 Feb 2022 at 13:07, wrote:
>
> From 99efcd655e83f034bce25271fe592d8789529e54 Mon Sep 17 00:00:00 2001
> From: Amir Gonnen
> Date: Thu, 17 Feb 2022 15:43:14 +0200
> Subject: [PATCH] target/nios2: Shadow register set, EIC and VIC
>
> Implement nios2 Vectored Interrupt Controller (VIC).
> This includes Exteral Interrupt Controller interface (EIC) and Shadow
> Register set implementation on the nios2 cpu.
> Implemented missing wrprs and rdprs instructions, and fixed eret.
> Added intc_present property, true by default. When set to false, nios2
> uses the EIC interface when handling IRQ.
Hi; this patch seems to be trying to fix multiple things at once. Could you
split it up into a multi-patch patch series, where each patch does one logical
thing, please? In particular bug fixes to existing code ("fixed eret") should
be their own patches, separate from patches adding new features.
> To use VIC, the nios2 board should set VIC cpu property, disable
> intc_present, connect VIC irq to cpu and connect VIC gpios.
Is there a patch which wires up one of the nios2 boards to do this ?
https://www.qemu.org/docs/master/devel/submitting-a-patch.html
is our guidelines on patch formatting.
thanks
-- PMM
The contents of this email message and any attachments are intended solely for
the addressee(s) and may contain confidential and/or privileged information and
may be legally protected from disclosure. If you are not the intended recipient
of this message or their agent, or if this message has been addressed to you in
error, please immediately alert the sender by reply email and then delete this
message and any attachments. If you are not the intended recipient, you are
hereby notified that any use, dissemination, copying, or storage of this
message or its attachments is strictly prohibited.
Re: [PATCH 01/11] mos6522: add defines for IFR bit flags
On Sun, 20 Feb 2022, Mark Cave-Ayland wrote: On 05/02/2022 12:06, BALATON Zoltan wrote: On Sat, 5 Feb 2022, Philippe Mathieu-Daudé wrote: On 5/2/22 11:51, Mark Cave-Ayland wrote: On 27/01/2022 23:16, BALATON Zoltan wrote: On Thu, 27 Jan 2022, Mark Cave-Ayland wrote: These are intended to make it easier to see how the physical control lines are wired for each instance. Signed-off-by: Mark Cave-Ayland --- include/hw/misc/mos6522.h | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h index fc95d22b0f..12abd8b8d2 100644 --- a/include/hw/misc/mos6522.h +++ b/include/hw/misc/mos6522.h @@ -41,13 +41,21 @@ #define IER_SET 0x80 /* set bits in IER */ #define IER_CLR 0 /* clear bits in IER */ -#define CA2_INT 0x01 -#define CA1_INT 0x02 -#define SR_INT 0x04 /* Shift register full/empty */ -#define CB2_INT 0x08 -#define CB1_INT 0x10 -#define T2_INT 0x20 /* Timer 2 interrupt */ -#define T1_INT 0x40 /* Timer 1 interrupt */ +#define CA2_INT_BIT 0 +#define CA1_INT_BIT 1 +#define SR_INT_BIT 2 /* Shift register full/empty */ +#define CB2_INT_BIT 3 +#define CB1_INT_BIT 4 +#define T2_INT_BIT 5 /* Timer 2 interrupt */ +#define T1_INT_BIT 6 /* Timer 1 interrupt */ + +#define CA2_INT (1 << CA2_INT_BIT) +#define CA1_INT (1 << CA1_INT_BIT) +#define SR_INT (1 << SR_INT_BIT) +#define CB2_INT (1 << CB2_INT_BIT) +#define CB1_INT (1 << CB1_INT_BIT) +#define T2_INT (1 << T2_INT_BIT) +#define T1_INT (1 << T1_INT_BIT) Maybe you could leave the #defines called XX_INT and then use BIT(XX_INT) instead of the second set of #defines which would provide same readability but with less #defines needed. I'm not so keen on removing the _INT defines since that would require updating all users to use BIT() everywhere which I don't think gains us much. I could certainly replace these definitions with BIT(FOO) instead of (1 << FOO) if that helps readability though. Do you mean simply doing this? -#define T1_INT 0x40 /* Timer 1 interrupt */ +#define T1_INT BIT(6) /* Timer 1 interrupt */ I meant: #define T1_INT 6 and then replace current usage of T1_INT with BIT(T1_INT) that way we don't need both T1_INT_BIT and T1_INT defines which seems redundant as BIT(T1_INT) is not much longer and still clear what it refers to. It's true that this needs more changes but the result is more readable IMO than introducing another set of defines that ome has to look up when encounters them as the meaning might not be clear. That's why I think one set of defines for bit numbers and using existing BIT(num) for values is better but it's just an idea, I don't care that much. I think the best solution here is to just use BIT() for the final shifted values like this: #define CA2_INT_BIT0 ... ... #define CA2_INTBIT(CA2_INT_BIT) That does not really help much as the idea was to avoid having two set of defines and only have one set for the bit numbers then use the BIT() macro instead of the current values. Using the BIT() macro in the second set of defines does not help reduce the number of defines in code which the reader will have to look up in this header. IMO having defines only for bit numbers and always using BIT(whatever) for values is less confusing assuming one is familiar with what the BIT() macro does. Otherwise I can see there being confusion given that the BIT() macro is used for defines without a _BIT suffix which are also being used elsewhere. Maybe it's only confusing to you as you've named the bit numbers *_BIT and the values without BIT and my proposal was to name the bit numbers as the simple names and use BIT(name) for the value which looks kind of opposite naming but it's the simplest. I guess you could also have bit numbers named *_BIT and then use BIT(CA2_INT_BIT) instead of the second set of defines for the values but that looks a bit redundant and maybe more confusing than just using BIT(CA2_INT). I'll update this in v2 accordingly. I don't have a strong opinion on this so if you prefer the way it is now or using the BIT() macro only for the separate defines then do that. I don't mind either way. Regards, BALATON Zoltan
[PATCH 0/2] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table
This can allow the guest OS to determine more easily if i8042 controller is present in the system or not, so it doesn't need to do probing of the controller, but just initialize it immediately, before enumerating the ACPI AML namespace. To allow "flexible" indication, I don't hardcode the bit at location 1 as on in the IA-PC boot flags, but try to search for i8042 on the ISA bus to verify it exists in the system. Why this is useful you might ask - this patch allows the guest OS to probe and use the i8042 controller without decoding the ACPI AML blob at all. For example, as a developer of the SerenityOS kernel, I might want to allow people to not try to decode the ACPI AML namespace (for now, we still don't support ACPI AML as it's a work in progress), but still to not probe for the i8042 but just use it after looking in the IA-PC boot flags in the ACPI FADT table. Liav Albani (2): hw/isa: add function to check for existence of device by its type hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table hw/acpi/aml-build.c | 7 ++- hw/i386/acpi-build.c| 5 + hw/i386/acpi-microvm.c | 5 + hw/isa/isa-bus.c| 17 + include/hw/acpi/acpi-defs.h | 1 + include/hw/isa/isa.h| 1 + 6 files changed, 35 insertions(+), 1 deletion(-) -- 2.35.1
Re: [PATCH] target/nios2: Shadow register set, EIC and VIC
On Sun, 20 Feb 2022 at 15:37, Amir Gonnen wrote: > I can split the VIC from the core+EIC changes, although the core+EIC changes > make little sense without a VIC to interact with them. > However, I'm not sure how to split the changes to the nios2 core into > multiple patches. > The shadow register set, together with the EIC interface and interrupt > handling are very much tied together. > > Regarding the "fixed eret" - perhaps I didn't phrase it right. What I meant > is that eret was changed to work correctly in the presence of a shadow > register set. > So, the changes to eret are part of the shadow register set support on the > core and cannot exist on their own. > > https://www.qemu.org/docs/master/devel/submitting-a-patch.html > > is our guidelines on patch formatting. > > In fact, I tried to follow them. I've also run checkpatch.pl, etc. > Could you please point out where I failed to follow them or what I'm missing? Mostly it was the combination of two things: (1) a large patch that touches both device and cpu code (2) a commit message that lists half a dozen things with very low level of detail This triggers my "probably needs to be split" heuristic, and also "probably somebody's first patch". So I mentioned the URL in case you hadn't seen it yet. > I tested this on Neuroblade board with JUART. I did not wire it to an > existing demo board. I think we'd like an upstream board that uses this. Otherwise it's dead code, from our point of view. That said, my suggested split would be something like: * VIC device model * CPU changes, in digestible chunks -- these should all be conditional on the "behave the same as the old code" default value of intc_present, I assume, so it doesn't matter if they don't all arrive in the codebase in the same commit. If there are any changes which *do* change behaviour even for intc_present=true, they definitely need to be split out, anyway. * board changes to use the new device thanks -- PMM
[PATCH 1/2] hw/isa: add function to check for existence of device by its type
Signed-off-by: Liav Albani
---
hw/isa/isa-bus.c | 17 +
include/hw/isa/isa.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 6c31398dda..39d1768797 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -222,6 +222,23 @@ void isa_build_aml(ISABus *bus, Aml *scope)
}
}
+bool isa_check_device_existence(const char *typename)
+{
+assert(isabus != NULL);
+
+BusChild *kid;
+ISADevice *dev;
+
+QTAILQ_FOREACH(kid, &isabus->parent_obj.children, sibling) {
+dev = ISA_DEVICE(kid->child);
+const char *object_type = object_get_typename(OBJECT(dev));
+if (object_type && strcmp(object_type, typename) == 0) {
+return true;
+}
+}
+return false;
+}
+
static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
{
ISADevice *d = ISA_DEVICE(dev);
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index d4417b34b6..65f0c7e28c 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -99,6 +99,7 @@ IsaDma *isa_get_dma(ISABus *bus, int nchan);
MemoryRegion *isa_address_space(ISADevice *dev);
MemoryRegion *isa_address_space_io(ISADevice *dev);
ISADevice *isa_new(const char *name);
+bool isa_check_device_existence(const char *typename);
ISADevice *isa_try_new(const char *name);
bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
ISADevice *isa_create_simple(ISABus *bus, const char *name);
--
2.35.1
[PATCH 2/2] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table
This can allow the guest OS to determine more easily if i8042 controller
is present in the system or not, so it doesn't need to do probing of the
controller, but just initialize it immediately, before enumerating the
ACPI AML namespace.
Signed-off-by: Liav Albani
---
hw/acpi/aml-build.c | 7 ++-
hw/i386/acpi-build.c| 5 +
hw/i386/acpi-microvm.c | 5 +
include/hw/acpi/acpi-defs.h | 1 +
4 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 8966e16320..ef5f4cad87 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2152,7 +2152,12 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const
AcpiFadtData *f,
build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
-build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
+/* IAPC_BOOT_ARCH */
+if (f->rev == 1) {
+build_append_int_noprefix(tbl, 0, 2);
+} else {
+build_append_int_noprefix(tbl, f->iapc_boot_arch, 2);
+}
build_append_int_noprefix(tbl, 0, 1); /* Reserved */
build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebd47aa26f..5dc625b8d8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -192,6 +192,11 @@ static void init_common_fadt_data(MachineState *ms, Object
*o,
.address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
},
};
+if (isa_check_device_existence("i8042")) {
+/* Indicates if i8042 is present or not */
+fadt.iapc_boot_arch = (1 << 1);
+}
+
*data = fadt;
}
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index 68ca7e7fc2..756c69b3b0 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -189,6 +189,11 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
.reset_val = ACPI_GED_RESET_VALUE,
};
+if (isa_check_device_existence("i8042")) {
+/* Indicates if i8042 is present or not */
+pmfadt.iapc_boot_arch = (1 << 1);
+}
+
table_offsets = g_array_new(false, true /* clear */,
sizeof(uint32_t));
bios_linker_loader_alloc(tables->linker,
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index c97e8633ad..2b42e4192b 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -77,6 +77,7 @@ typedef struct AcpiFadtData {
uint16_t plvl2_lat;/* P_LVL2_LAT */
uint16_t plvl3_lat;/* P_LVL3_LAT */
uint16_t arm_boot_arch;/* ARM_BOOT_ARCH */
+uint16_t iapc_boot_arch; /* IAPC_BOOT_ARCH */
uint8_t minor_ver; /* FADT Minor Version */
/*
--
2.35.1
Re: [PULL 00/39] ppc queue
On Fri, 18 Feb 2022 at 10:38, Cédric Le Goater wrote: > > The following changes since commit c13b8e9973635f34f3ce4356af27a311c993729c: > > Merge remote-tracking branch > 'remotes/alistair/tags/pull-riscv-to-apply-20220216' into staging (2022-02-16 > 09:57:11 +) > > are available in the Git repository at: > > https://github.com/legoater/qemu/ tags/pull-ppc-20220218 > > for you to fetch changes up to 65e0446c86ee70d2125c1f1d1e36e6c2dfb08642: > > target/ppc: Move common SPR functions out of cpu_init (2022-02-18 08:34:15 > +0100) > > > ppc-7.0 queue > > * target/ppc: SPR registration cleanups (Fabiano) > * ppc: nested KVM HV for spapr virtual hypervisor (Nicholas) > * spapr: nvdimm: Introduce spapr-nvdimm device (Shivaprasad) > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0 for any user-visible changes. -- PMM
Re: [PATCH v3 4/6] hw/openrisc/openrisc_sim: Increase max_cpus to 4
On 19/2/22 07:42, Stafford Horne wrote: Now that we no longer have a limit of 2 CPUs due to fixing the IRQ routing issues we can increase the max. Here we increase the limit to 4, we could go higher, but currently OMPIC has a limit of 4, so we align with that. Signed-off-by: Stafford Horne --- hw/openrisc/openrisc_sim.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 3/6] hw/openrisc/openrisc_sim: Use IRQ splitter when connecting UART
On 19/2/22 07:42, Stafford Horne wrote:
Currently the OpenRISC SMP configuration only supports 2 cores due to
the UART IRQ routing being limited to 2 cores. As was done in commit
1eeffbeb11 ("hw/openrisc/openrisc_sim: Use IRQ splitter when connecting
IRQ to multiple CPUs") we can use a splitter to wire more than 2 CPUs.
This patch moves serial initialization out to it's own function and
uses a splitter to connect multiple CPU irq lines to the UART.
Signed-off-by: Stafford Horne
---
hw/openrisc/openrisc_sim.c | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index d12b3e0c5e..5bfbac00f8 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -137,6 +137,28 @@ static void openrisc_sim_ompic_init(hwaddr base, int
num_cpus,
sysbus_mmio_map(s, 0, base);
}
+static void openrisc_sim_serial_init(hwaddr base, int num_cpus,
+ OpenRISCCPU *cpus[], int irq_pin)
+{
+qemu_irq serial_irq;
+int i;
+
+if (num_cpus > 1) {
+DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
+qdev_prop_set_uint32(splitter, "num-lines", num_cpus);
+qdev_realize_and_unref(splitter, NULL, &error_fatal);
+for (i = 0; i < num_cpus; i++) {
+qdev_connect_gpio_out(splitter, i, get_cpu_irq(cpus, i, irq_pin));
+}
+serial_irq = qdev_get_gpio_in(splitter, 0);
+} else {
+serial_irq = get_cpu_irq(cpus, 0, irq_pin);
+}
Up to here the code seems a generic helper:
or1k_cpus_connect_device(OpenRISCCPU *cpus[],
unsigned num_cpus,
unsigned irq_pin);
+serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
+ serial_hd(0), DEVICE_NATIVE_ENDIAN);
This part specific to UART.
+}
Anyhow,
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v8 04/11] 9p: darwin: Handle struct dirent differences
On Sonntag, 20. Februar 2022 17:50:49 CET Will Cohen wrote:
> From: Keno Fischer
>
> On darwin d_seekoff exists, but is optional and does not seem to
> be commonly used by file systems. Use `telldir` instead to obtain
> the seek offset and inject it into d_seekoff, and create a
> qemu_dirent_off helper to call it appropriately when appropriate.
>
> Signed-off-by: Keno Fischer
> [Michael Roitzsch: - Rebase for NixOS]
> Signed-off-by: Michael Roitzsch
> [Will Cohen: - Adjust to pass testing
> - Ensure that d_seekoff is filled using telldir
>on darwin, and create qemu_dirent_off helper
>to decide which to access]
> [Fabian Franz: - Add telldir error handling for darwin]
> Signed-off-by: Fabian Franz
> [Will Cohen: - Ensure that telldir error handling uses
>signed int
> - Cleanup of telldir error handling
> - Remove superfluous error handling for
>qemu_dirent_off
> - Adjust formatting
> - Use qemu_dirent_off in codir.c]
> Signed-off-by: Will Cohen
> ---
This patch does not compile on Linux ...
> hw/9pfs/9p-local.c | 9 +
> hw/9pfs/9p-proxy.c | 16 +++-
> hw/9pfs/9p-synth.c | 4
> hw/9pfs/9p-util.h | 16
> hw/9pfs/9p.c | 7 +--
> hw/9pfs/codir.c| 4 +++-
> 6 files changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 1a5e3eed73..f3272f0b43 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -562,6 +562,15 @@ again:
> if (!entry) {
> return NULL;
> }
> +#ifdef CONFIG_DARWIN
> +int off;
> +off = telldir(fs->dir.stream);
> +/* If telldir fails, fail the entire readdir call */
> +if (off < 0) {
> +return NULL;
> +}
> +entry->d_seekoff = off;
> +#endif
>
> if (ctx->export_flags & V9FS_SM_MAPPED) {
> entry->d_type = DT_UNKNOWN;
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index b1664080d8..8b4b5cf7dc 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx,
> V9fsFidOpenState *fs)
>
> static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState *fs)
> {
> -return readdir(fs->dir.stream);
> +struct dirent *entry;
> +entry = readdir(fs->dir.stream);
> +#ifdef CONFIG_DARWIN
> +if (!entry) {
> +return NULL;
> +}
> +int td;
> +td = telldir(fs->dir.stream);
> +/* If telldir fails, fail the entire readdir call */
> +if (td < 0) {
> +return NULL;
> +}
> +entry->d_seekoff = td;
> +#endif
> +return entry;
> }
>
> static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off)
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index bf9b0c5ddd..b3080e415b 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node,
> offsetof(struct dirent, d_name) + sz);
> memcpy(entry->d_name, node->name, sz);
> entry->d_ino = node->attr->inode;
> +#ifdef CONFIG_DARWIN
> +entry->d_seekoff = off + 1;
> +#else
> entry->d_off = off + 1;
> +#endif
> }
>
> static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
> diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> index 546f46dc7d..d41f37f085 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -79,3 +79,19 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char
> *filename, const char *name);
>
> #endif
... ^- this is the end of file #endif, so qemu_dirent_off() should be above
that #endif, and ...
> +
> +
> +/**
> + * Darwin has d_seekoff, which appears to function similarly to d_off.
> + * However, it does not appear to be supported on all file systems,
> + * so ensure it is manually injected earlier and call here when
> + * needed.
> + */
> +inline off_t qemu_dirent_off(struct dirent *dent)
... this function declaration misses the 'static' keyword, which is mandatory
to prevent a linker error.
Best regards,
Christian Schoenebeck
> +{
> +#ifdef CONFIG_DARWIN
> +return dent->d_seekoff;
> +#else
> +return dent->d_off;
> +#endif
> +}
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 1563d7b7c6..caf3b240fe 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -27,6 +27,7 @@
> #include "virtio-9p.h"
> #include "fsdev/qemu-fsdev.h"
> #include "9p-xattr.h"
> +#include "9p-util.h"
> #include "coth.h"
> #include "trace.h"
> #include "migration/blocker.h"
> @@ -2281,7 +2282,7 @@ static int coroutine_fn
> v9fs_do_readdir_with_stat(V9fsPDU *pdu, count += len;
> v9fs_stat_free(&v9stat);
> v9fs_path_free(&path);
> -saved_dir_pos = dent->d_off;
> +saved_dir_pos = qemu_dirent_off(dent);
> }
>
> v9fs_readdir_unlock(&fidp->fs.dir);
> @@ -2420,6 +2421,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu,
> V9fsFidState
Re: [PATCH v8 04/11] 9p: darwin: Handle struct dirent differences
Apologies — I tested on Mac but should have done Linux too. Will revise.
On Sun, Feb 20, 2022 at 4:28 PM Christian Schoenebeck <
qemu_...@crudebyte.com> wrote:
> On Sonntag, 20. Februar 2022 17:50:49 CET Will Cohen wrote:
> > From: Keno Fischer
> >
> > On darwin d_seekoff exists, but is optional and does not seem to
> > be commonly used by file systems. Use `telldir` instead to obtain
> > the seek offset and inject it into d_seekoff, and create a
> > qemu_dirent_off helper to call it appropriately when appropriate.
> >
> > Signed-off-by: Keno Fischer
> > [Michael Roitzsch: - Rebase for NixOS]
> > Signed-off-by: Michael Roitzsch
> > [Will Cohen: - Adjust to pass testing
> > - Ensure that d_seekoff is filled using telldir
> >on darwin, and create qemu_dirent_off helper
> >to decide which to access]
> > [Fabian Franz: - Add telldir error handling for darwin]
> > Signed-off-by: Fabian Franz
> > [Will Cohen: - Ensure that telldir error handling uses
> >signed int
> > - Cleanup of telldir error handling
> > - Remove superfluous error handling for
> >qemu_dirent_off
> > - Adjust formatting
> > - Use qemu_dirent_off in codir.c]
> > Signed-off-by: Will Cohen
> > ---
>
> This patch does not compile on Linux ...
>
> > hw/9pfs/9p-local.c | 9 +
> > hw/9pfs/9p-proxy.c | 16 +++-
> > hw/9pfs/9p-synth.c | 4
> > hw/9pfs/9p-util.h | 16
> > hw/9pfs/9p.c | 7 +--
> > hw/9pfs/codir.c| 4 +++-
> > 6 files changed, 52 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index 1a5e3eed73..f3272f0b43 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -562,6 +562,15 @@ again:
> > if (!entry) {
> > return NULL;
> > }
> > +#ifdef CONFIG_DARWIN
> > +int off;
> > +off = telldir(fs->dir.stream);
> > +/* If telldir fails, fail the entire readdir call */
> > +if (off < 0) {
> > +return NULL;
> > +}
> > +entry->d_seekoff = off;
> > +#endif
> >
> > if (ctx->export_flags & V9FS_SM_MAPPED) {
> > entry->d_type = DT_UNKNOWN;
> > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> > index b1664080d8..8b4b5cf7dc 100644
> > --- a/hw/9pfs/9p-proxy.c
> > +++ b/hw/9pfs/9p-proxy.c
> > @@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx,
> > V9fsFidOpenState *fs)
> >
> > static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState
> *fs)
> > {
> > -return readdir(fs->dir.stream);
> > +struct dirent *entry;
> > +entry = readdir(fs->dir.stream);
> > +#ifdef CONFIG_DARWIN
> > +if (!entry) {
> > +return NULL;
> > +}
> > +int td;
> > +td = telldir(fs->dir.stream);
> > +/* If telldir fails, fail the entire readdir call */
> > +if (td < 0) {
> > +return NULL;
> > +}
> > +entry->d_seekoff = td;
> > +#endif
> > +return entry;
> > }
> >
> > static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t
> off)
> > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > index bf9b0c5ddd..b3080e415b 100644
> > --- a/hw/9pfs/9p-synth.c
> > +++ b/hw/9pfs/9p-synth.c
> > @@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node,
> > offsetof(struct dirent, d_name) + sz);
> > memcpy(entry->d_name, node->name, sz);
> > entry->d_ino = node->attr->inode;
> > +#ifdef CONFIG_DARWIN
> > +entry->d_seekoff = off + 1;
> > +#else
> > entry->d_off = off + 1;
> > +#endif
> > }
> >
> > static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > index 546f46dc7d..d41f37f085 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -79,3 +79,19 @@ ssize_t fremovexattrat_nofollow(int dirfd, const char
> > *filename, const char *name);
> >
> > #endif
>
> ... ^- this is the end of file #endif, so qemu_dirent_off() should be
> above
> that #endif, and ...
>
> > +
> > +
> > +/**
> > + * Darwin has d_seekoff, which appears to function similarly to d_off.
> > + * However, it does not appear to be supported on all file systems,
> > + * so ensure it is manually injected earlier and call here when
> > + * needed.
> > + */
> > +inline off_t qemu_dirent_off(struct dirent *dent)
>
> ... this function declaration misses the 'static' keyword, which is
> mandatory
> to prevent a linker error.
>
> Best regards,
> Christian Schoenebeck
>
> > +{
> > +#ifdef CONFIG_DARWIN
> > +return dent->d_seekoff;
> > +#else
> > +return dent->d_off;
> > +#endif
> > +}
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 1563d7b7c6..caf3b240fe 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -27,6 +27,7 @@
> > #include "virtio-9p.h"
> > #include "fsdev/qemu-fsdev.h"
> > #include "9p-xattr.h"
> > +#include "9p-util.h"
> > #include "coth.h"
> > #include "trace.h"
Re: [PATCH v8 09/11] 9p: darwin: Implement compatibility for mknodat
On Sonntag, 20. Februar 2022 17:50:54 CET Will Cohen wrote:
> From: Keno Fischer
>
> Darwin does not support mknodat. However, to avoid race conditions
> with later setting the permissions, we must avoid using mknod on
> the full path instead. We could try to fchdir, but that would cause
> problems if multiple threads try to call mknodat at the same time.
> However, luckily there is a solution: Darwin includes a function
> that sets the cwd for the current thread only.
> This should suffice to use mknod safely.
>
> This function (pthread_fchdir_np) is protected by a check in
> meson in a patch later in this series.
>
> Signed-off-by: Keno Fischer
> Signed-off-by: Michael Roitzsch
> [Will Cohen: - Adjust coding style
> - Replace clang references with gcc
> - Note radar filed with Apple for missing syscall
> - Replace direct syscall with pthread_fchdir_np and
>adjust patch notes accordingly
> - Move qemu_mknodat from 9p-util to osdep and os-posix
> - Move pthread_fchdir_np declaration only to osdep
> - Declare pthread_fchdir_np with
> - __attribute__((weak_import)) to allow checking for
>its presence before usage
> - Move declarations above cplusplus guard
> - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
>presence in osdep.h and os-posix.c
> - Rebase to apply cleanly on top of the 2022-02-10
>changes to 9pfs]
> Signed-off-by: Will Cohen
> ---
> hw/9pfs/9p-local.c | 4 ++--
> include/qemu/osdep.h | 12
> meson.build | 1 +
> os-posix.c | 35 +++
> 4 files changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index a0d08e5216..d42ce6d8b8 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> *dir_path,
>
> if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
> fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
> -err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> +err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
> if (err == -1) {
> goto out;
> }
> @@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath
> *dir_path, }
> } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
> fs_ctx->export_flags & V9FS_SM_NONE) {
> -err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> +err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
> if (err == -1) {
> goto out;
> }
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index ce12f64853..c0f442d791 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -818,6 +818,18 @@ static inline int
> platform_does_not_support_system(const char *command) */
> struct dirent *qemu_dirent_dup(struct dirent *dent);
>
> +/*
> + * As long as mknodat is not available on macOS, this workaround
> + * using pthread_fchdir_np is needed. qemu_mknodat is defined in
> + * os-posix.c. pthread_fchdir_np is weakly linked here as a guard
> + * in case it disappears in future macOS versions, because it is
> + * is a private API.
> + */
> +#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
> +int pthread_fchdir_np(int fd) __attribute__((weak_import));
> +#endif
> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/meson.build b/meson.build
> index ae5f7eec6e..6fdc0281ad 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1557,6 +1557,7 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE',
> cc.has_function('posix_fallocate'
> config_host_data.set('CONFIG_POSIX_MEMALIGN',
> cc.has_function('posix_memalign')) config_host_data.set('CONFIG_PPOLL',
> cc.has_function('ppoll'))
> config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
> '#include ')) +config_host_data.set('CONFIG_PTHREAD_FCHDIR_NP',
> cc.has_function('pthread_fchdir_np'))
> config_host_data.set('CONFIG_SEM_TIMEDWAIT',
> cc.has_function('sem_timedwait', dependencies: threads))
> config_host_data.set('CONFIG_SENDFILE', cc.has_function('sendfile'))
> config_host_data.set('CONFIG_SETNS', cc.has_function('setns') and
> cc.has_function('unshare')) diff --git a/os-posix.c b/os-posix.c
> index ae6c9f2a5e..ccc3d1e9d3 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -332,3 +332,38 @@ int os_mlock(void)
> return -ENOSYS;
> #endif
> }
> +
> +/*
> + * As long as mknodat is not available on macOS, this workaround
> + * using pthread_fchdir_np is needed.
> + *
> + * Radar filed with Apple for implementing mknodat:
> + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> + */
> +#if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP
> +
>
Re: [PATCH v3 4/6] hw/openrisc/openrisc_sim: Increase max_cpus to 4
On Sun, Feb 20, 2022 at 09:07:17PM +0100, Philippe Mathieu-Daudé wrote: > On 19/2/22 07:42, Stafford Horne wrote: > > Now that we no longer have a limit of 2 CPUs due to fixing the > > IRQ routing issues we can increase the max. Here we increase > > the limit to 4, we could go higher, but currently OMPIC has a > > limit of 4, so we align with that. > > > > Signed-off-by: Stafford Horne > > --- > > hw/openrisc/openrisc_sim.c | 8 +--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > Reviewed-by: Philippe Mathieu-Daudé Thank you. -Stafford
Re: [PATCH v3 3/6] hw/openrisc/openrisc_sim: Use IRQ splitter when connecting UART
On Sun, Feb 20, 2022 at 09:06:41PM +0100, Philippe Mathieu-Daudé wrote:
> On 19/2/22 07:42, Stafford Horne wrote:
> > Currently the OpenRISC SMP configuration only supports 2 cores due to
> > the UART IRQ routing being limited to 2 cores. As was done in commit
> > 1eeffbeb11 ("hw/openrisc/openrisc_sim: Use IRQ splitter when connecting
> > IRQ to multiple CPUs") we can use a splitter to wire more than 2 CPUs.
> >
> > This patch moves serial initialization out to it's own function and
> > uses a splitter to connect multiple CPU irq lines to the UART.
> >
> > Signed-off-by: Stafford Horne
> > ---
> > hw/openrisc/openrisc_sim.c | 32
> > 1 file changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> > index d12b3e0c5e..5bfbac00f8 100644
> > --- a/hw/openrisc/openrisc_sim.c
> > +++ b/hw/openrisc/openrisc_sim.c
> > @@ -137,6 +137,28 @@ static void openrisc_sim_ompic_init(hwaddr base, int
> > num_cpus,
> > sysbus_mmio_map(s, 0, base);
> > }
> > +static void openrisc_sim_serial_init(hwaddr base, int num_cpus,
> > + OpenRISCCPU *cpus[], int irq_pin)
> > +{
> > +qemu_irq serial_irq;
> > +int i;
> > +
> > +if (num_cpus > 1) {
> > +DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
> > +qdev_prop_set_uint32(splitter, "num-lines", num_cpus);
> > +qdev_realize_and_unref(splitter, NULL, &error_fatal);
> > +for (i = 0; i < num_cpus; i++) {
> > +qdev_connect_gpio_out(splitter, i, get_cpu_irq(cpus, i,
> > irq_pin));
> > +}
> > +serial_irq = qdev_get_gpio_in(splitter, 0);
> > +} else {
> > +serial_irq = get_cpu_irq(cpus, 0, irq_pin);
> > +}
>
> Up to here the code seems a generic helper:
>
> or1k_cpus_connect_device(OpenRISCCPU *cpus[],
>unsigned num_cpus,
>unsigned irq_pin);
Right, this is similar to that used in openrisc_sim_net_init. I thought about
sharing the code but I didn't think it worth adding helper.
The main reason for me is that openrisc_sim_net_init doesn't expose the qemu_irq
and just does sysbus_connect_irq. While openrisc_sim_serial_init exposes the
qemu_irq.
I think a generic function would have to look like:
qemu_irq openrisc_cpus_irq_pin_init(OpenRISCCPU *cpus[],
unsigned num_cpus,
unsigned irq_pin);
I would like to leave this as is for now as.
> > +serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
> > + serial_hd(0), DEVICE_NATIVE_ENDIAN);
>
> This part specific to UART.
Right.
> > +}
>
> Anyhow,
> Reviewed-by: Philippe Mathieu-Daudé
Thank you,
Adding this to the patch as is.
-Stafford
Re: [RFC PATCH] virtio-net: Unlimit tx queue size if peer is vdpa
在 2022/2/18 上午1:50, Eugenio Pérez 写道:
The code used to limit the maximum size of tx queue for others backends
than vhost_user since the introduction of configurable tx queue size in
9b02e1618cf2 ("virtio-net: enable configurable tx queue size").
As vhost_user, vhost_vdpa devices should deal with memory region
crosses already, so let's use the full tx size.
Signed-off-by: Eugenio Pérez
Acked-by: Jason Wang
---
hw/net/virtio-net.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 49cd13314a..b1769bfee0 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -629,17 +629,20 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n)
NetClientState *peer = n->nic_conf.peers.ncs[0];
/*
- * Backends other than vhost-user don't support max queue size.
+ * Backends other than vhost-user or vhost-vdpa don't support max queue
+ * size.
*/
if (!peer) {
return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
}
-if (peer->info->type != NET_CLIENT_DRIVER_VHOST_USER) {
+switch(peer->info->type) {
+case NET_CLIENT_DRIVER_VHOST_USER:
+case NET_CLIENT_DRIVER_VHOST_VDPA:
+return VIRTQUEUE_MAX_SIZE;
+default:
return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
-}
-
-return VIRTQUEUE_MAX_SIZE;
+};
}
static int peer_attach(VirtIONet *n, int index)
[PATCH] gdbstub.c: add support for info proc mappings
This commit adds support for `info proc mappings` and a few other commands into
the QEMU user-mode emulation gdbstub.
For that, support for the following GDB remote protocol commands has been added:
* vFile:setfs: pid
* vFile:open: filename, flags, mode
* vFile:pread: fd, count, offset
* vFile:close: fd
* qXfer:exec-file:read:annex:offset,length
Additionally, a `REMOTE_DEBUG_PRINT` macro has been added for printing remote
debug logs.
To enable it, set the `ENABLE_REMOTE_DEBUG` definition to 1.
All this can be tested with the following steps (commands from Ubuntu 20.04):
1. Compiling an example program, e.g. for ARM:
echo 'int main() {puts("Hello world");}' | arm-linux-gnueabihf-gcc -xc -
2. Running qemu-arm with gdbstub:
qemu-arm -g 1234 -L /usr/arm-linux-gnueabihf/ ./a.out
3. Connecting to the gdbstub with GDB:
gdb-multiarch -ex 'target remote localhost:1234'
4. Executing `info proc mappings` in GDB.
The opening of files is done on behalf of the inferior by reusing the do_openat
syscall.
Note that the current solution is still imperfect: while it allows us to fetch
procfs
files like /proc/$pid/maps in the same way as the inferior is seeing them,
there are
two downsides to it. First of all, it is indeed performed on behalf of the
inferior.
Second of all, there are some files that the GDB tries to request like
/lib/libc.so.6,
but they are usually not available as they do not exist in those paths and may
need to
be served from the prefix provided in the -L flag to the qemu-user binary. I
may try
to add a support for this in another patch and maybe refactor the solution to
not use
the do_openat function directly.
Before this commit, one would get (except of amd64, but not i386 targets):
```
(gdb) info proc mappings
process 1
warning: unable to open /proc file '/proc/1/maps'
```
And after this commit, we should get something like:
```
(gdb) info proc mappings
process 3167519
Mapped address spaces:
Start Addr End Addr Size Offset objfile
0x3f7d 0x3f7d1000 0x10000x0
0x3f7d1000 0x3f7ed0000x1c0000x0
/usr/arm-linux-gnueabihf/lib/ld-2.33.so
0x3f7ed000 0x3f7fd0000x10x0
0x3f7fd000 0x3f7ff000 0x20000x1c000
/usr/arm-linux-gnueabihf/lib/ld-2.33.so
0x3f7ff000 0x3f80 0x10000x0
0x3f80 0x4000 0x800x0 [stack]
0x4000 0x40001000 0x10000x0 /home/dc/src/qemu/a.out
0x40001000 0x4001 0xf0000x0
0x4001 0x40012000 0x20000x0 /home/dc/src/qemu/a.out
```
However, on amd64 targets we would get and still get the following on the GDB
side
(even after this commit):
```
(gdb) info proc mappings
Not supported on this target.
```
The x64 behavior is related to the fact that the GDB client does not initialize
some of its remote handlers properly when the gdbstub does not send an
"orig_rax"
register in the target.xml file that describes the target. This happens in GDB
in the
amd64_linux_init_abi function in the amd64-linux-tdep.c file [0]. The GDB tries
to find
the "org.gnu.gdb.i386.linux" feature and the "orig_rax" register in it and if
it is not
present, then it does not proceed with the `amd64_linux_init_abi_common (info,
gdbarch, 2);` call
which initializes whatever is needed so that GDB fetches `info proc mappings`
properly.
I tried to fix this but just adding the orig_rax registry into the target.xml
did not work
(there was some mismatch between the expected and sent register values; I guess
the QEMU stub
would need to know how to send this register's value). On the other hand, this
could also be
fixed on the GDB side. I will discuss this with GDB maintainers or/and propose
a patch to GDB
related to this.
[0]
https://github.com/bminor/binutils-gdb/blob/dc5483c989f29fc9c7934965071ae1bb80cff902/gdb/amd64-linux-tdep.c#L1863-L1873
Signed-off-by: Dominik 'Disconnect3d' Czarnota
---
gdbstub.c| 272 +++
linux-user/qemu.h| 2 +
linux-user/syscall.c | 2 +-
3 files changed, 275 insertions(+), 1 deletion(-)
diff --git a/gdbstub.c b/gdbstub.c
index 3c14c6a038..69cf8bbb0c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -34,6 +34,10 @@
#include "exec/gdbstub.h"
#ifdef CONFIG_USER_ONLY
#include "qemu.h"
+#ifdef CONFIG_LINUX
+#include "linux-user/qemu.h"
+#include "linux-user/loader.h"
+#endif
#else
#include "monitor/monitor.h"
#include "chardev/char.h"
@@ -62,6 +66,21 @@
static int phy_memory_mode;
#endif
+/*
+ * Set to 1 to enable remote protocol debugging output. This output is similar
+ * to the one produced by the gdbserver's --remote-debug flag with some
+ * additions. Anyway, the main debug prints are:
+ * - getpkt ("...") which refers to received data (or, send by the GDB client)
+ * - putpkt ("...") which refers to sent data
+ */
+#define ENABLE_REMOTE_DEBUG 0
+
+#if ENABLE_REMOTE_DEBUG
+#define REMOTE_DEB
Re: [PATCH 2/2] Allow VIRTIO_F_IN_ORDER to be negotiated for vdpa devices
在 2022/2/18 下午6:22, Eugenio Perez Martin 写道:
On Thu, Feb 17, 2022 at 8:32 AM Michael S. Tsirkin wrote:
On Tue, Feb 15, 2022 at 12:52:31PM +0530, Gautam Dawar wrote:
This patch adds the ability to negotiate VIRTIO_F_IN_ORDER bit
for vhost-vdpa backend when the underlying device supports this
feature.
This would aid in reaping performance benefits with HW devices
that implement this feature. At the same time, it shouldn't have
any negative impact as vhost-vdpa backend doesn't involve any
userspace virtqueue operations.
Signed-off-by: Gautam Dawar
Having features that hardware implements but qemu does not
means we can't migrate between them.
So I'd rather see a userspace implementation.
While I totally agree the userspace implementation is a better option,
would it be a problem if we implement it as a cmdline option as Jason
proposed?
I see other backends have similar issues with migration. For example
it's possible to run qemu with
-device=virtio-net-pci,...,indirect_desc=on and use a vhost-kernel
backend without indirect support in their features. I also understand
qemu emulated backend as "the base" somehow, but it should work
similarly to my example if cmdline parameter is off by default.
We had a lot of issues with the command line compatibility like this
(e.g qemu may silently clear a host feature) which is probably worth to
fix in the future.
On the other hand, It may be worth thinking if it's worth waiting for
GSoC though, so we avoid this problem entirely at the moment. But I
feel that is going to come back with a different feature set with the
advent of more out of qemu devices and the fast adding of features of
VirtIO.
Thoughts?
Not sure we had sufficient resources on the Qemu/kernel implementation
of in-order. But if we decide not to wait we can change the GSoC to
implement other stuffs like e.g NOTIFICATION_DATA.
Thanks
Thanks!
---
hw/net/virtio-net.c | 10 ++
net/vhost-vdpa.c| 1 +
2 files changed, 11 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index cf8ab0f8af..a1089d06f6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3507,11 +3507,21 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
nc->rxfilter_notify_enabled = 1;
if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+uint64_t features = BIT_ULL(VIRTIO_F_IN_ORDER);
struct virtio_net_config netcfg = {};
+
memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
vhost_net_set_config(get_vhost_net(nc->peer),
(uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_MASTER);
+
+ /*
+ * For vhost-vdpa, if underlying device supports IN_ORDER feature,
+ * make it available for negotiation.
+ */
+ features = vhost_net_get_features(get_vhost_net(nc->peer), features);
+ n->host_features |= features;
}
+
QTAILQ_INIT(&n->rsc_chains);
n->qdev = dev;
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 25dd6dd975..2886cba5ec 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -62,6 +62,7 @@ const int vdpa_feature_bits[] = {
VIRTIO_NET_F_CTRL_VQ,
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_PACKED,
+VIRTIO_F_IN_ORDER,
VIRTIO_NET_F_RSS,
VIRTIO_NET_F_HASH_REPORT,
VIRTIO_NET_F_GUEST_ANNOUNCE,
--
2.30.1
Re: Call for GSoC and Outreachy project ideas for summer 2022
On Jan 28 15:47, Stefan Hajnoczi wrote: > Dear QEMU, KVM, and rust-vmm communities, > QEMU will apply for Google Summer of Code 2022 > (https://summerofcode.withgoogle.com/) and has been accepted into > Outreachy May-August 2022 (https://www.outreachy.org/). You can now > submit internship project ideas for QEMU, KVM, and rust-vmm! > > If you have experience contributing to QEMU, KVM, or rust-vmm you can > be a mentor. It's a great way to give back and you get to work with > people who are just starting out in open source. > > Please reply to this email by February 21st with your project ideas. > > Good project ideas are suitable for remote work by a competent > programmer who is not yet familiar with the codebase. In > addition, they are: > - Well-defined - the scope is clear > - Self-contained - there are few dependencies > - Uncontroversial - they are acceptable to the community > - Incremental - they produce deliverables along the way > > Feel free to post ideas even if you are unable to mentor the project. > It doesn't hurt to share the idea! > > I will review project ideas and keep you up-to-date on QEMU's > acceptance into GSoC. > > Internship program details: > - Paid, remote work open source internships > - GSoC projects are 175 or 350 hours, Outreachy projects are 30 > hrs/week for 12 weeks > - Mentored by volunteers from QEMU, KVM, and rust-vmm > - Mentors typically spend at least 5 hours per week during the coding period > > Changes since last year: GSoC now has 175 or 350 hour project sizes > instead of 12 week full-time projects. GSoC will accept applicants who > are not students, before it was limited to students. > > For more background on QEMU internships, check out this video: > https://www.youtube.com/watch?v=xNVCX7YMUL8 > > Please let me know if you have any questions! > > Stefan > Hi, I'd like to revive the "NVMe Performance" proposal from Paolo and Stefan from two years ago. https://wiki.qemu.org/Internships/ProjectIdeas/NVMePerformance I'd like to mentor, but since this is "iothread-heavy", I'd like to be able to draw a bit on Stefan, Paolo if possible. Klaus signature.asc Description: PGP signature
Re: [PATCH RFCv2 2/4] i386/pc: relocate 4g start to 1T where applicable
On Fri, 18 Feb 2022 17:12:21 +
Joao Martins wrote:
> On 2/14/22 15:31, Igor Mammedov wrote:
> > On Mon, 14 Feb 2022 15:05:00 +
> > Joao Martins wrote:
> >> On 2/14/22 14:53, Igor Mammedov wrote:
> >>> On Mon, 7 Feb 2022 20:24:20 +
> >>> Joao Martins wrote:
> +{
> +PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> +X86MachineState *x86ms = X86_MACHINE(pcms);
> +ram_addr_t device_mem_size = 0;
> +uint32_t eax, vendor[3];
> +
> +host_cpuid(0x0, 0, &eax, &vendor[0], &vendor[2], &vendor[1]);
> +if (!IS_AMD_VENDOR(vendor)) {
> +return;
> +}
> +
> +if (pcmc->has_reserved_memory &&
> + (machine->ram_size < machine->maxram_size)) {
> +device_mem_size = machine->maxram_size - machine->ram_size;
> +}
> +
> +if ((x86ms->above_4g_mem_start + x86ms->above_4g_mem_size +
> + device_mem_size) < AMD_HT_START) {
> >>>
> >> And I was at two minds on this one, whether to advertise *always*
> >> the 1T hole, regardless of relocation. Or account the size
> >> we advertise for the pci64 hole and make that part of the equation
> >> above. Although that has the flaw that the firmware at admin request
> >> may pick some ludricous number (limited by maxphysaddr).
> >
> > it this point we have only pci64 hole size (machine property),
> > so I'd include that in equation to make firmware assign
> > pci64 aperture above HT range.
> >
> > as for checking maxphysaddr, we can only check 'default' PCI hole
> > range at this stage (i.e. 1Gb aligned hole size after all possible RAM)
> > and hard error on it.
> >
>
> Igor, in the context of your comment above, I'll be introducing another
> preparatory patch that adds up pci_hole64_size to pc_memory_init() such
> that all used/max physaddr space checks are consolidated in pc_memory_init().
>
> To that end, the changes involve mainly moves the the pcihost qdev creation
> to be before pc_memory_init(). Q35 just needs a 2-line order change. i440fx
> needs slightly more of a dance to extract that from i440fx_init() and also
> because most i440fx state is private (hence the new helper for size). But
> the actual initialization of I440fx/q35 pci host is still after
> pc_memory_init(),
> it is just to extra pci_hole64_size from the object + user passed args
> (-global etc).
Shuffling init order is looks too intrusive and in practice
quite risky.
How about moving maxphysaddr check to pc_machine_done() instead?
(this way you won't have to move pcihost around)
> Raw staging changes below the scissors mark so far.
>
> -->8--
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b2e43eba1106..902977081350 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -875,7 +875,8 @@ static void x86_update_above_4g_mem_start(PCMachineState
> *pcms)
> void pc_memory_init(PCMachineState *pcms,
> MemoryRegion *system_memory,
> MemoryRegion *rom_memory,
> -MemoryRegion **ram_memory)
> +MemoryRegion **ram_memory,
> +uint64_t pci_hole64_size)
> {
> int linux_boot, i;
> MemoryRegion *option_rom_mr;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d9b344248dac..5a608e30e28f 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -91,6 +91,8 @@ static void pc_init1(MachineState *machine,
> MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> ram_addr_t lowmem;
> +uint64_t hole64_size;
> +DeviceState *i440fx_dev;
>
> /*
> * Calculate ram split, for memory below and above 4G. It's a bit
> @@ -164,9 +166,13 @@ static void pc_init1(MachineState *machine,
> pci_memory = g_new(MemoryRegion, 1);
> memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> rom_memory = pci_memory;
> +i440fx_dev = qdev_new(host_type);
> +hole64_size = i440fx_pci_hole64_size(i440fx_dev);
> } else {
> pci_memory = NULL;
> rom_memory = system_memory;
> +i440fx_dev = NULL;
> +hole64_size = 0;
> }
>
> pc_guest_info_init(pcms);
> @@ -183,7 +189,7 @@ static void pc_init1(MachineState *machine,
> /* allocate ram and load rom/bios */
> if (!xen_enabled()) {
> pc_memory_init(pcms, system_memory,
> - rom_memory, &ram_memory);
> + rom_memory, &ram_memory, hole64_size);
> } else {
> pc_system_flash_cleanup_unused(pcms);
> if (machine->kernel_filename != NULL) {
> @@ -199,7 +205,7 @@ static void pc_init1(MachineState *machine,
>
> pci_bus = i440fx_init(host_type,
>pci_type,
> - &i440fx_state,
> + i440fx_dev, &i440fx_state,
>system_memory, system_io, machine->ram_size,
>
Re: [PATCH v3 2/3] hw/smbios: fix table memory corruption with large memory vms
On Thu, 17 Feb 2022 19:02:05 +0530 (IST) Ani Sinha wrote: > On Thu, 17 Feb 2022, Igor Mammedov wrote: > > > On Mon, 14 Feb 2022 19:42:35 +0530 > > > > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2023977 > > s/buglink/Resolves/ > > > > OK I am curious about this one. > > Per https://www.qemu.org/docs/master/devel/submitting-a-patch.html , > > If your patch fixes a bug in the gitlab bug tracker, please add a line > with “Resolves: ” to the commit message, too. Gitlab can > close bugs automatically once commits with the “Resolved:” keyword get > merged into the master branch of the project. And if your patch addresses > a bug in another public bug tracker, you can also use a line with > “Buglink: ” for reference here, too. > > So I considered redhar BZ as a public bug tracker as well. Does the BZ > also automatically close bugs when we use "Resolves:"? Should we update > the instructions here? I might have been wrong suggesting "Resolves" tag, it will/should not affect bugzilla. Looking at history, shows a zoo of used tags in bugzilla case, so I'd guess Buglink is fine as any other.
Re: [PATCH v10 0/5] QEMU RISC-V AIA support
On Sun, Feb 20, 2022 at 6:57 PM Anup Patel wrote: > > From: Anup Patel > > The advanced interrupt architecture (AIA) extends the per-HART local > interrupt support. Along with this, it also adds IMSIC (MSI contrllor) > and Advanced PLIC (wired interrupt controller). > > The latest AIA draft specification can be found here: > https://github.com/riscv/riscv-aia/releases/download/0.2-draft.28/riscv-interrupts-028.pdf > > This series adds RISC-V AIA support in QEMU which includes emulating all > AIA local CSRs, APLIC, and IMSIC. Only AIA local interrupt filtering is > not implemented because we don't have any local interrupt greater than 12. > > To enable AIA in QEMU, use one of the following: > 1) Only AIA local interrupt CSRs: Pass "x-aia=true" as CPU paramenter >in the QEMU command-line > 2) Only APLIC for virt machine: Pass "aia=aplic" as machine parameter >in the QEMU command-line > 3) Both APLIC and IMSIC for virt machine: Pass "aia=aplic-imsic" as >machine parameter in the QEMU command-line > 4) Both APLIC and IMSIC with 2 guest files for virt machine: Pass >"aia=aplic-imsic,aia-guests=2" as machine parameter in the QEMU >command-line > > To test series, we require Linux with AIA support which can be found in: > riscv_aia_v1 branch at https://github.com/avpatel/linux.git > > This series can be found riscv_aia_v10 branch at: > https://github.com/avpatel/qemu.git > > Changes since v9: > - Rebased on latest riscv-to-apply.next branch of Alistair's repo > - Removed first 18 PATCHs since these are already merged > - Fixed 32-bit system compile error in PATCH3 > > Changes since v8: > - Use error_setg() in riscv_imsic_realize() added by PATCH20 > > Changes since v7: > - Rebased on latest riscv-to-apply.next branch of Alistair's repo > - Improved default priority assignment in PATCH9 > > Changes since v6: > - Fixed priority comparison in riscv_cpu_pending_to_irq() of PATCH9 > - Fixed typos in comments added by PATCH11 > - Added "pend = true;" for CSR_MSETEIPNUM case of rmw_xsetclreinum() >in PATCH15 > - Handle ithreshold == 0 case in riscv_aplic_idc_topi() of PATCH18 > - Allow setting pending bit for Level0 or Level1 interrupts in >riscv_aplic_set_pending() of PATCH18 > - Force DOMAINCFG[31:24] bits to 0x80 in riscv_aplic_read() of PATCH18 > - For APLIC direct mode, set target.iprio to 1 when zero is writtern >in PATCH18 > - Handle eithreshold == 0 case in riscv_imsic_topei() of PATCH20 > > Changes since v5: > - Moved VSTOPI_NUM_SRCS define to top of the file in PATCH13 > - Fixed typo in PATCH16 > > Changes since v4: > - Changed IRQ_LOCAL_MAX to 16 in PATCH2 > - Fixed typo in PATCH10 > - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH11 > - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH14 > - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH15 > - Replaced TARGET_LONG_BITS with xlen passed via ireg callback in PATCH20 > - Retrict maximum IMSIC guest files per-HART of virt machine to 7 in >PATCH21. > - Added separate PATCH23 to increase maximum number of allowed CPUs >for virt machine > > Changes since v3: > - Replaced "aplic,xyz" and "imsic,xyz" DT properties with "riscv,xyz" >DT properties because "aplic" and "imsic" are not valid vendor names >required by Linux DT schema checker. > > Changes since v2: > - Update PATCH4 to check and inject interrupt after V=1 when >transitioning from V=0 to V=1 > > Changes since v1: > - Revamped whole series and created more granular patches > - Added HGEIE and HGEIP CSR emulation for H-extension > - Added APLIC emulation > - Added IMSIC emulation > > Anup Patel (5): > hw/riscv: virt: Add optional AIA APLIC support to virt machine > hw/intc: Add RISC-V AIA IMSIC device emulation > hw/riscv: virt: Add optional AIA IMSIC support to virt machine > docs/system: riscv: Document AIA options for virt machine > hw/riscv: virt: Increase maximum number of allowed CPUs Thanks! Applied to riscv-to-apply.next Alistair > > docs/system/riscv/virt.rst| 16 + > hw/intc/Kconfig | 3 + > hw/intc/meson.build | 1 + > hw/intc/riscv_imsic.c | 448 ++ > hw/riscv/Kconfig | 2 + > hw/riscv/virt.c | 698 -- > include/hw/intc/riscv_imsic.h | 68 > include/hw/riscv/virt.h | 41 +- > 8 files changed, 1156 insertions(+), 121 deletions(-) > create mode 100644 hw/intc/riscv_imsic.c > create mode 100644 include/hw/intc/riscv_imsic.h > > -- > 2.25.1 > >
Re: [PATCH v2] hw: riscv: opentitan: fixup SPI addresses
On Fri, Feb 18, 2022 at 4:38 PM Alistair Francis
wrote:
>
> From: Wilfred Mallawa
>
> This patch updates the SPI_DEVICE, SPI_HOST0, SPI_HOST1
> base addresses. Also adds these as unimplemented devices.
>
> The address references can be found [1].
>
> [1]
> https://github.com/lowRISC/opentitan/blob/6c317992fbd646818b34f2a2dbf44bc850e461e4/hw/top_earlgrey/sw/autogen/top_earlgrey_memory.h#L107
>
> Signed-off-by: Wilfred Mallawa
> Reviewed-by: Alistair Francis
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> v2: arranged base addrs in sorted order
>
> hw/riscv/opentitan.c | 12 +---
> include/hw/riscv/opentitan.h | 4 +++-
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index aec7cfa33f..833624d66c 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -34,13 +34,15 @@ static const MemMapEntry ibex_memmap[] = {
> [IBEX_DEV_FLASH] = { 0x2000, 0x8 },
> [IBEX_DEV_UART] = { 0x4000, 0x1000 },
> [IBEX_DEV_GPIO] = { 0x4004, 0x1000 },
> -[IBEX_DEV_SPI] ={ 0x4005, 0x1000 },
> +[IBEX_DEV_SPI_DEVICE] = { 0x4005, 0x1000 },
> [IBEX_DEV_I2C] ={ 0x4008, 0x1000 },
> [IBEX_DEV_PATTGEN] ={ 0x400e, 0x1000 },
> [IBEX_DEV_TIMER] = { 0x4010, 0x1000 },
> [IBEX_DEV_SENSOR_CTRL] ={ 0x4011, 0x1000 },
> [IBEX_DEV_OTP_CTRL] = { 0x4013, 0x4000 },
> [IBEX_DEV_USBDEV] = { 0x4015, 0x1000 },
> +[IBEX_DEV_SPI_HOST0] = { 0x4030, 0x1000 },
> +[IBEX_DEV_SPI_HOST1] = { 0x4031, 0x1000 },
> [IBEX_DEV_PWRMGR] = { 0x4040, 0x1000 },
> [IBEX_DEV_RSTMGR] = { 0x4041, 0x1000 },
> [IBEX_DEV_CLKMGR] = { 0x4042, 0x1000 },
> @@ -209,8 +211,12 @@ static void lowrisc_ibex_soc_realize(DeviceState
> *dev_soc, Error **errp)
>
> create_unimplemented_device("riscv.lowrisc.ibex.gpio",
> memmap[IBEX_DEV_GPIO].base, memmap[IBEX_DEV_GPIO].size);
> -create_unimplemented_device("riscv.lowrisc.ibex.spi",
> -memmap[IBEX_DEV_SPI].base, memmap[IBEX_DEV_SPI].size);
> +create_unimplemented_device("riscv.lowrisc.ibex.spi_device",
> +memmap[IBEX_DEV_SPI_DEVICE].base, memmap[IBEX_DEV_SPI_DEVICE].size);
> +create_unimplemented_device("riscv.lowrisc.ibex.spi_host0",
> +memmap[IBEX_DEV_SPI_HOST0].base, memmap[IBEX_DEV_SPI_HOST0].size);
> +create_unimplemented_device("riscv.lowrisc.ibex.spi_host1",
> +memmap[IBEX_DEV_SPI_HOST1].base, memmap[IBEX_DEV_SPI_HOST1].size);
> create_unimplemented_device("riscv.lowrisc.ibex.i2c",
> memmap[IBEX_DEV_I2C].base, memmap[IBEX_DEV_I2C].size);
> create_unimplemented_device("riscv.lowrisc.ibex.pattgen",
> diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
> index eac35ef590..00da9ded43 100644
> --- a/include/hw/riscv/opentitan.h
> +++ b/include/hw/riscv/opentitan.h
> @@ -57,8 +57,10 @@ enum {
> IBEX_DEV_FLASH,
> IBEX_DEV_FLASH_VIRTUAL,
> IBEX_DEV_UART,
> +IBEX_DEV_SPI_DEVICE,
> +IBEX_DEV_SPI_HOST0,
> +IBEX_DEV_SPI_HOST1,
> IBEX_DEV_GPIO,
> -IBEX_DEV_SPI,
> IBEX_DEV_I2C,
> IBEX_DEV_PATTGEN,
> IBEX_DEV_TIMER,
> --
> 2.35.1
>
Re: [PATCH 17/31] vdpa: adapt vhost_ops callbacks to svq
在 2022/2/18 上午1:13, Eugenio Perez Martin 写道:
On Tue, Feb 8, 2022 at 4:58 AM Jason Wang wrote:
在 2022/2/1 上午2:58, Eugenio Perez Martin 写道:
On Sun, Jan 30, 2022 at 5:03 AM Jason Wang wrote:
在 2022/1/22 上午4:27, Eugenio Pérez 写道:
First half of the buffers forwarding part, preparing vhost-vdpa
callbacks to SVQ to offer it. QEMU cannot enable it at this moment, so
this is effectively dead code at the moment, but it helps to reduce
patch size.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.h | 2 +-
hw/virtio/vhost-shadow-virtqueue.c | 21 -
hw/virtio/vhost-vdpa.c | 133 ++---
3 files changed, 143 insertions(+), 13 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
index 035207a469..39aef5ffdf 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -35,7 +35,7 @@ size_t vhost_svq_device_area_size(const VhostShadowVirtqueue
*svq);
void vhost_svq_stop(VhostShadowVirtqueue *svq);
-VhostShadowVirtqueue *vhost_svq_new(void);
+VhostShadowVirtqueue *vhost_svq_new(uint16_t qsize);
void vhost_svq_free(VhostShadowVirtqueue *vq);
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index f129ec8395..7c168075d7 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -277,9 +277,17 @@ void vhost_svq_stop(VhostShadowVirtqueue *svq)
/**
* Creates vhost shadow virtqueue, and instruct vhost device to use the
shadow
* methods and file descriptors.
+ *
+ * @qsize Shadow VirtQueue size
+ *
+ * Returns the new virtqueue or NULL.
+ *
+ * In case of error, reason is reported through error_report.
*/
-VhostShadowVirtqueue *vhost_svq_new(void)
+VhostShadowVirtqueue *vhost_svq_new(uint16_t qsize)
{
+size_t desc_size = sizeof(vring_desc_t) * qsize;
+size_t device_size, driver_size;
g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
int r;
@@ -300,6 +308,15 @@ VhostShadowVirtqueue *vhost_svq_new(void)
/* Placeholder descriptor, it should be deleted at set_kick_fd */
event_notifier_init_fd(&svq->svq_kick, INVALID_SVQ_KICK_FD);
+svq->vring.num = qsize;
I wonder if this is the best. E.g some hardware can support up to 32K
queue size. So this will probably end up with:
1) SVQ use 32K queue size
2) hardware queue uses 256
In that case SVQ vring queue size will be 32K and guest's vring can
negotiate any number with SVQ equal or less than 32K,
Sorry for being unclear what I meant is actually
1) SVQ uses 32K queue size
2) guest vq uses 256
This looks like a burden that needs extra logic and may damage the
performance.
Still not getting this point.
An available guest buffer, although contiguous in GPA/GVA, can expand
in multiple buffers if it's not contiguous in qemu's VA (by the while
loop in virtqueue_map_desc [1]). In that scenario it is better to have
"plenty" of SVQ buffers.
Yes, but this case should be rare. So in this case we should deal with
overrun on SVQ, that is
1) SVQ is full
2) guest VQ isn't
We need to
1) check the available buffer slots
2) disable guest kick and wait for the used buffers
But it looks to me the current code is not ready for dealing with this case?
I'm ok if we decide to put an upper limit though, or if we decide not
to handle this situation. But we would leave out valid virtio drivers.
Maybe to set a fixed upper limit (1024?)? To add another parameter
(x-svq-size-n=N)?
If you mean we lose performance because memory gets more sparse I
think the only possibility is to limit that way.
If guest is not using 32K, having a 32K for svq may gives extra stress
on the cache since we will end up with a pretty large working set.
And this can lead other interesting situation:
1) SVQ uses 256
2) guest vq uses 1024
Where a lot of more SVQ logic is needed.
If we agree that a guest descriptor can expand in multiple SVQ
descriptors, this should be already handled by the previous logic too.
But this should only happen in case that qemu is launched with a "bad"
cmdline, isn't it?
This seems can happen when we use -device
virtio-net-pci,tx_queue_size=1024 with a 256 size vp_vdpa device at least?
If I run that example with vp_vdpa, L0 qemu will happily accept 1024
as a queue size [2]. But if the vdpa device maximum queue size is
effectively 256, this will result in an error: We're not exposing it
to the guest at any moment but with qemu's cmdline.
including 256.
Is that what you mean?
I mean, it looks to me the logic will be much more simplified if we just
allocate the shadow virtqueue with the size what guest can see (guest
vring).
Then we don't need to think if the difference of the queue size can have
any side effects.
I think that we cannot avoid that extra logic unless we force GPA to
be contiguous in IOVA. If we are sure the guest's buffe
Re: [PATCH 01/31] vdpa: Reorder virtio/vhost-vdpa.c functions
在 2022/1/28 下午3:57, Eugenio Perez Martin 写道:
On Fri, Jan 28, 2022 at 6:59 AM Jason Wang wrote:
在 2022/1/22 上午4:27, Eugenio Pérez 写道:
vhost_vdpa_set_features and vhost_vdpa_init need to use
vhost_vdpa_get_features in svq mode.
vhost_vdpa_dev_start needs to use almost all _set_ functions:
vhost_vdpa_set_vring_dev_kick, vhost_vdpa_set_vring_dev_call,
vhost_vdpa_set_dev_vring_base and vhost_vdpa_set_dev_vring_num.
No functional change intended.
Is it related (a must) to the SVQ code?
Yes, SVQ needs to access the device variants to configure it, while
exposing the SVQ ones.
For example for set_features, SVQ needs to set device features in the
start code, but expose SVQ ones to the guest.
Another possibility is to forward-declare them but I feel it pollutes
the code more, doesn't it? Is there any reason to avoid the reordering
beyond reducing the number of changes/patches?
No, but for reviewer, it might be easier if you squash the reordering
logic into the patch which needs that.
Thanks
Thanks!
Thanks
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-vdpa.c | 164 -
1 file changed, 82 insertions(+), 82 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 04ea43704f..6c10a7f05f 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -342,41 +342,6 @@ static bool vhost_vdpa_one_time_request(struct vhost_dev
*dev)
return v->index != 0;
}
-static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
-{
-struct vhost_vdpa *v;
-assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
-trace_vhost_vdpa_init(dev, opaque);
-int ret;
-
-/*
- * Similar to VFIO, we end up pinning all guest memory and have to
- * disable discarding of RAM.
- */
-ret = ram_block_discard_disable(true);
-if (ret) {
-error_report("Cannot set discarding of RAM broken");
-return ret;
-}
-
-v = opaque;
-v->dev = dev;
-dev->opaque = opaque ;
-v->listener = vhost_vdpa_memory_listener;
-v->msg_type = VHOST_IOTLB_MSG_V2;
-
-vhost_vdpa_get_iova_range(v);
-
-if (vhost_vdpa_one_time_request(dev)) {
-return 0;
-}
-
-vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
- VIRTIO_CONFIG_S_DRIVER);
-
-return 0;
-}
-
static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
int queue_index)
{
@@ -506,24 +471,6 @@ static int vhost_vdpa_set_mem_table(struct vhost_dev *dev,
return 0;
}
-static int vhost_vdpa_set_features(struct vhost_dev *dev,
- uint64_t features)
-{
-int ret;
-
-if (vhost_vdpa_one_time_request(dev)) {
-return 0;
-}
-
-trace_vhost_vdpa_set_features(dev, features);
-ret = vhost_vdpa_call(dev, VHOST_SET_FEATURES, &features);
-if (ret) {
-return ret;
-}
-
-return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
-}
-
static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev)
{
uint64_t features;
@@ -646,35 +593,6 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev,
uint8_t *config,
return ret;
}
-static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
-{
-struct vhost_vdpa *v = dev->opaque;
-trace_vhost_vdpa_dev_start(dev, started);
-
-if (started) {
-vhost_vdpa_host_notifiers_init(dev);
-vhost_vdpa_set_vring_ready(dev);
-} else {
-vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
-}
-
-if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
-return 0;
-}
-
-if (started) {
-memory_listener_register(&v->listener, &address_space_memory);
-return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
-} else {
-vhost_vdpa_reset_device(dev);
-vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
- VIRTIO_CONFIG_S_DRIVER);
-memory_listener_unregister(&v->listener);
-
-return 0;
-}
-}
-
static int vhost_vdpa_set_log_base(struct vhost_dev *dev, uint64_t base,
struct vhost_log *log)
{
@@ -735,6 +653,35 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
}
+static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
+{
+struct vhost_vdpa *v = dev->opaque;
+trace_vhost_vdpa_dev_start(dev, started);
+
+if (started) {
+vhost_vdpa_host_notifiers_init(dev);
+vhost_vdpa_set_vring_ready(dev);
+} else {
+vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
+}
+
+if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
+return 0;
+}
+
+if (started) {
+memory_listener_register(&v->listener, &address_space_memory);
+r
Re: [PATCH 09/31] vhost-vdpa: Take into account SVQ in vhost_vdpa_set_vring_call
在 2022/2/18 下午8:35, Eugenio Perez Martin 写道:
On Tue, Feb 8, 2022 at 4:23 AM Jason Wang wrote:
在 2022/1/31 下午11:34, Eugenio Perez Martin 写道:
On Sat, Jan 29, 2022 at 9:06 AM Jason Wang wrote:
在 2022/1/22 上午4:27, Eugenio Pérez 写道:
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-vdpa.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 18de14f0fb..029f98feee 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -687,13 +687,29 @@ static int vhost_vdpa_set_vring_kick(struct vhost_dev
*dev,
}
}
-static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
- struct vhost_vring_file *file)
+static int vhost_vdpa_set_vring_dev_call(struct vhost_dev *dev,
+ struct vhost_vring_file *file)
{
trace_vhost_vdpa_set_vring_call(dev, file->index, file->fd);
return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
}
+static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
+ struct vhost_vring_file *file)
+{
+struct vhost_vdpa *v = dev->opaque;
+
+if (v->shadow_vqs_enabled) {
+int vdpa_idx = vhost_vdpa_get_vq_index(dev, file->index);
+VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, vdpa_idx);
+
+vhost_svq_set_guest_call_notifier(svq, file->fd);
Two questions here (had similar questions for vring kick):
1) Any reason that we setup the eventfd for vhost-vdpa in
vhost_vdpa_svq_setup() not here?
I'm not sure what you mean.
The guest->SVQ call and kick fds are set here and at
vhost_vdpa_set_vring_kick. The event notifier handler of the guest ->
SVQ kick_fd is set at vhost_vdpa_set_vring_kick /
vhost_svq_set_svq_kick_fd. The guest -> SVQ call fd has no event
notifier handler since we don't poll it.
On the other hand, the connection SVQ <-> device uses the same fds
from the beginning to the end, and they will not change with, for
example, call fd masking. That's why it's setup from
vhost_vdpa_svq_setup. Delaying to vhost_vdpa_set_vring_call would make
us add way more logic there.
More logic in general shadow vq code but less codes for vhost-vdpa
specific code I think.
E.g for we can move the kick set logic from vhost_vdpa_svq_set_fds() to
here.
But they are different fds. vhost_vdpa_svq_set_fds sets the
SVQ<->device. This function sets the SVQ->guest call file descriptor.
To move the logic of vhost_vdpa_svq_set_fds here would imply either:
a) Logic to know if we are receiving the first call fd or not.
Any reason for this? I guess you meant multiqueue. If yes, it should not
be much difference since we have idx as the parameter.
That
code is not in the series at the moment, because setting at
vhost_vdpa_dev_start tells the difference for free. Is just adding
code, not moving.
b) Logic to set again *the same* file descriptor to device, with logic
to tell if we have missed calls. That logic is not implemented for
device->SVQ call file descriptor, because we are assuming it never
changes from vhost_vdpa_svq_set_fds. So this is again adding code.
At this moment, we have:
vhost_vdpa_svq_set_fds:
set SVQ<->device fds
vhost_vdpa_set_vring_call:
set guest<-SVQ call
vhost_vdpa_set_vring_kick:
set guest->SVQ kick.
If I understood correctly, the alternative would be something like:
vhost_vdpa_set_vring_call:
set guest<-SVQ call
if(!vq->call_set) {
- set SVQ<-device call.
- vq->call_set = true
}
vhost_vdpa_set_vring_kick:
set guest<-SVQ call
if(!vq->dev_kick_set) {
- set guest->device kick.
- vq->dev_kick_set = true
}
dev_reset / dev_stop:
for vq in vqs:
vq->dev_kick_set = vq->dev_call_set = false
...
Or have I misunderstood something?
I wonder what happens if MSI-X is masking in guest. So if I understand
correctly, we don't disable the eventfd from device? If yes, this seems
suboptinal.
Thanks
Thanks!
Thanks
2) The call could be disabled by using -1 as the fd, I don't see any
code to deal with that.
Right, I didn't take that into account. vhost-kernel takes also -1 as
kick_fd to unbind, so SVQ can be reworked to take that into account
for sure.
Thanks!
Thanks
+return 0;
+} else {
+return vhost_vdpa_set_vring_dev_call(dev, file);
+}
+}
+
/**
* Set shadow virtqueue descriptors to the device
*
Re: [PATCH 01/31] vdpa: Reorder virtio/vhost-vdpa.c functions
On Mon, Feb 21, 2022 at 8:31 AM Jason Wang wrote:
>
>
> 在 2022/1/28 下午3:57, Eugenio Perez Martin 写道:
> > On Fri, Jan 28, 2022 at 6:59 AM Jason Wang wrote:
> >>
> >> 在 2022/1/22 上午4:27, Eugenio Pérez 写道:
> >>> vhost_vdpa_set_features and vhost_vdpa_init need to use
> >>> vhost_vdpa_get_features in svq mode.
> >>>
> >>> vhost_vdpa_dev_start needs to use almost all _set_ functions:
> >>> vhost_vdpa_set_vring_dev_kick, vhost_vdpa_set_vring_dev_call,
> >>> vhost_vdpa_set_dev_vring_base and vhost_vdpa_set_dev_vring_num.
> >>>
> >>> No functional change intended.
> >>
> >> Is it related (a must) to the SVQ code?
> >>
> > Yes, SVQ needs to access the device variants to configure it, while
> > exposing the SVQ ones.
> >
> > For example for set_features, SVQ needs to set device features in the
> > start code, but expose SVQ ones to the guest.
> >
> > Another possibility is to forward-declare them but I feel it pollutes
> > the code more, doesn't it? Is there any reason to avoid the reordering
> > beyond reducing the number of changes/patches?
>
>
> No, but for reviewer, it might be easier if you squash the reordering
> logic into the patch which needs that.
>
Sure, I can do that way. I thought the opposite but I can merge the
reorder in the different patches for the next version for sure.
Thanks!
> Thanks
>
>
> >
> > Thanks!
> >
> >
> >> Thanks
> >>
> >>
> >>> Signed-off-by: Eugenio Pérez
> >>> ---
> >>>hw/virtio/vhost-vdpa.c | 164 -
> >>>1 file changed, 82 insertions(+), 82 deletions(-)
> >>>
> >>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> >>> index 04ea43704f..6c10a7f05f 100644
> >>> --- a/hw/virtio/vhost-vdpa.c
> >>> +++ b/hw/virtio/vhost-vdpa.c
> >>> @@ -342,41 +342,6 @@ static bool vhost_vdpa_one_time_request(struct
> >>> vhost_dev *dev)
> >>>return v->index != 0;
> >>>}
> >>>
> >>> -static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error
> >>> **errp)
> >>> -{
> >>> -struct vhost_vdpa *v;
> >>> -assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
> >>> -trace_vhost_vdpa_init(dev, opaque);
> >>> -int ret;
> >>> -
> >>> -/*
> >>> - * Similar to VFIO, we end up pinning all guest memory and have to
> >>> - * disable discarding of RAM.
> >>> - */
> >>> -ret = ram_block_discard_disable(true);
> >>> -if (ret) {
> >>> -error_report("Cannot set discarding of RAM broken");
> >>> -return ret;
> >>> -}
> >>> -
> >>> -v = opaque;
> >>> -v->dev = dev;
> >>> -dev->opaque = opaque ;
> >>> -v->listener = vhost_vdpa_memory_listener;
> >>> -v->msg_type = VHOST_IOTLB_MSG_V2;
> >>> -
> >>> -vhost_vdpa_get_iova_range(v);
> >>> -
> >>> -if (vhost_vdpa_one_time_request(dev)) {
> >>> -return 0;
> >>> -}
> >>> -
> >>> -vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> >>> - VIRTIO_CONFIG_S_DRIVER);
> >>> -
> >>> -return 0;
> >>> -}
> >>> -
> >>>static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
> >>>int queue_index)
> >>>{
> >>> @@ -506,24 +471,6 @@ static int vhost_vdpa_set_mem_table(struct vhost_dev
> >>> *dev,
> >>>return 0;
> >>>}
> >>>
> >>> -static int vhost_vdpa_set_features(struct vhost_dev *dev,
> >>> - uint64_t features)
> >>> -{
> >>> -int ret;
> >>> -
> >>> -if (vhost_vdpa_one_time_request(dev)) {
> >>> -return 0;
> >>> -}
> >>> -
> >>> -trace_vhost_vdpa_set_features(dev, features);
> >>> -ret = vhost_vdpa_call(dev, VHOST_SET_FEATURES, &features);
> >>> -if (ret) {
> >>> -return ret;
> >>> -}
> >>> -
> >>> -return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
> >>> -}
> >>> -
> >>>static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev)
> >>>{
> >>>uint64_t features;
> >>> @@ -646,35 +593,6 @@ static int vhost_vdpa_get_config(struct vhost_dev
> >>> *dev, uint8_t *config,
> >>>return ret;
> >>> }
> >>>
> >>> -static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> >>> -{
> >>> -struct vhost_vdpa *v = dev->opaque;
> >>> -trace_vhost_vdpa_dev_start(dev, started);
> >>> -
> >>> -if (started) {
> >>> -vhost_vdpa_host_notifiers_init(dev);
> >>> -vhost_vdpa_set_vring_ready(dev);
> >>> -} else {
> >>> -vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
> >>> -}
> >>> -
> >>> -if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
> >>> -return 0;
> >>> -}
> >>> -
> >>> -if (started) {
> >>> -memory_listener_register(&v->listener, &address_space_memory);
> >>> -return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> >>> -} else {
> >>> -vhost_vdpa_reset_device(dev);
> >>> -vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> >>> -
Re: [PATCH 18/31] vhost: Shadow virtqueue buffers forwarding
在 2022/2/17 下午8:48, Eugenio Perez Martin 写道:
On Tue, Feb 8, 2022 at 9:16 AM Jason Wang wrote:
在 2022/2/1 下午7:25, Eugenio Perez Martin 写道:
On Sun, Jan 30, 2022 at 7:47 AM Jason Wang wrote:
在 2022/1/22 上午4:27, Eugenio Pérez 写道:
@@ -272,6 +590,28 @@ void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq,
int svq_kick_fd)
void vhost_svq_stop(VhostShadowVirtqueue *svq)
{
event_notifier_set_handler(&svq->svq_kick, NULL);
+g_autofree VirtQueueElement *next_avail_elem = NULL;
+
+if (!svq->vq) {
+return;
+}
+
+/* Send all pending used descriptors to guest */
+vhost_svq_flush(svq, false);
Do we need to wait for all the pending descriptors to be completed here?
No, this function does not wait, it only completes the forwarding of
the *used* descriptors.
The best example is the net rx queue in my opinion. This call will
check SVQ's vring used_idx and will forward the last used descriptors
if any, but all available descriptors will remain as available for
qemu's VQ code.
To skip it would miss those last rx descriptors in migration.
Thanks!
So it's probably to not the best place to ask. It's more about the
inflight descriptors so it should be TX instead of RX.
I can imagine the migration last phase, we should stop the vhost-vDPA
before calling vhost_svq_stop(). Then we should be fine regardless of
inflight descriptors.
I think I'm still missing something here.
To be on the same page. Regarding tx this could cause repeated tx
frames (one at source and other at destination), but never a missed
buffer not transmitted. The "stop before" could be interpreted as "SVQ
is not forwarding available buffers anymore". Would that work?
Right, but this only work if
1) a flush to make sure TX DMA for inflight descriptors are all completed
2) just mark all inflight descriptor used
Otherwise there could be buffers that is inflight forever.
Thanks
Thanks!
Thanks
Thanks
+
+for (unsigned i = 0; i < svq->vring.num; ++i) {
+g_autofree VirtQueueElement *elem = NULL;
+elem = g_steal_pointer(&svq->ring_id_maps[i]);
+if (elem) {
+virtqueue_detach_element(svq->vq, elem, elem->len);
+}
+}
+
+next_avail_elem = g_steal_pointer(&svq->next_guest_avail_elem);
+if (next_avail_elem) {
+virtqueue_detach_element(svq->vq, next_avail_elem,
+ next_avail_elem->len);
+}
}
