Re: [PATCH v2 2/2] hw/ide: add ich6 ide controller device emulation
On 2/19/22 02:50, BALATON Zoltan wrote:
+/*
+ * QEMU IDE Emulation: PCI ICH6/ICH7 IDE support.
This is a small thing, but if these two are the same maybe keeping
this comment but using the ich7 name everywhere else would make it
less likely to get it confused with ich9. I mean ich6 and ich9 is
easily confused, while ich7 is clearly distinct. But maybe it's just
me, calling it ich6 is also correct and can be preferred by someone else.
ICH6 and ICH7 IDE controllers are quite the same as far as I know. I
could change it, but then one could argue that the name ich6-ide seems
like "ich9-ide", so not sure if we can really go on this path.
+static void ich6_pci_config_write(PCIDevice *d, uint32_t addr,
uint32_t val,
+ int l)
+{
+ uint32_t i;
+
+ pci_default_write_config(d, addr, val, l);
+
+ for (i = addr; i < addr + l; i++) {
+ switch (i) {
+ case 0x40:
+ pci_default_write_config(d, i, 0x8000, 2);
+ continue;
+ case 0x42:
+ pci_default_write_config(d, i, 0x8000, 2);
+ continue;
+ }
+ }
I'm not sure what this tries to do but this for cycle looks suspicious
here. It's also only calls pci_default_write_config() so why didn't
the default worked and why was this override needed?
It's just a loop to ensure that if the guest OS tries to disable an IDE
channel from the PCI config space, it seems "stuck" on enabled. I should
probably put a comment on this to explain this, but I definitely don't
want the guest OS to be able to disable any IDE channel for now (on real
hardware, it does that, but I think we can either skip this feature or
implement in a future patch, as Linux deals with the controller just fine).
+}
+
+static void ich6_ide_reset(DeviceState *dev)
+{
+ PCIIDEState *d = PCI_IDE(dev);
+ PCIDevice *pd = PCI_DEVICE(d);
+ uint8_t *pci_conf = pd->config;
+ int i;
+
+ for (i = 0; i < 2; i++) {
+ ide_bus_reset(&d->bus[i]);
+ }
+
+ /* TODO: this is the default. do not override. */
+ pci_conf[PCI_COMMAND] = 0x00;
+ /* TODO: this is the default. do not override. */
+ pci_conf[PCI_COMMAND + 1] = 0x00;
+ /* TODO: use pci_set_word */
+ pci_conf[PCI_STATUS] = PCI_STATUS_FAST_BACK;
+ pci_conf[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_MEDIUM >> 8;
+ pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
+}
+
+static int pci_ich6_init_ports(PCIIDEState *d)
+{
+ int i;
+
+ for (i = 0; i < 2; i++) {
+ ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
+ ide_init2(&d->bus[i], d->native_irq);
+
+ bmdma_init(&d->bus[i], &d->bmdma[i], d);
+ d->bmdma[i].bus = &d->bus[i];
+ ide_register_restart_cb(&d->bus[i]);
+ }
+
+ return 0;
+}
+
+static void pci_ich6_ide_realize(PCIDevice *dev, Error **errp)
+{
+ PCIIDEState *d = PCI_IDE(dev);
+ uint8_t *pci_conf = dev->config;
+ int rc;
All in all this device looks very similar to piix ide devices with
only some differentces so I wonder if ir could be implemented as
another device in piix.c. We already have 3 variants there: piix3-ide,
piix3-ide-xen, and piix4-ide so maybe putting this device in piix.c
could avoid some code duplication. In that case moving out
bmdma_{read,write} were not needed either although maybe that's a good
idea anyway to share it with other devices.
As for putting the ich6-ide code in piix.c - I'm not against it.
Although, in the long run, if I send more patches to QEMU, I rather
split the files a bit more in the /hw/ide directory as there's a lot of
code duplication to solve.
So, what we could do instead, is to share more code between the devices
and still keep them in separate files.
As for moving out the bmdma_{write,read} functions - I'm still in the
previous mind that we need to move them out as via.c shares the same
code but currently has code duplications for it, and that I want to
solve as part of making the IDE code more clean and to add more features.
However, if it seems necessary to do this cleanup before implementing
this ich6-ide controller, I'm more than happy to wait on this, send a
patch to solve and clean up some issues in the IDE code, and then
re-send this as v3.
I might even consider doing that now if nobody rejects this idea :)
+
+ pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
+
+ /* PCI native mode-only controller, supports bus mastering */
+ pci_conf[PCI_CLASS_PROG] = 0x85;
+
+ bmdma_setup_bar(d);
+ pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+
+ d->native_irq = pci_allocate_irq(&d->parent_obj);
Is this irq and the new field in PCIIDEState really needed? If this
device is using PCI interrupts could it do the same as CMD646 ide does
instead?
I looked into how cmd646.c does that, but it creates that with the
qdev_init_gpio_in function. The AHCI controller seems to use
pci_allocate_irq function (in ich.c), as well as other ha
[PATCH v3 6/6] hw/openrisc/openrisc_sim: Add support for initrd loading
The initrd passed via the command line is loaded into memory. It's
location and size is then added to the device tree so the kernel knows
where to find it.
Signed-off-by: Stafford Horne
Reviewed-by: Peter Maydell
---
hw/openrisc/openrisc_sim.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index e0e71c0faa..8184caa60b 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -315,6 +315,33 @@ static hwaddr openrisc_load_kernel(ram_addr_t ram_size,
return 0;
}
+static hwaddr openrisc_load_initrd(Or1ksimState *state, const char *filename,
+ hwaddr load_start, uint64_t mem_size)
+{
+void *fdt = state->fdt;
+int size;
+hwaddr start;
+
+/* We put the initrd right after the kernel; page aligned. */
+start = TARGET_PAGE_ALIGN(load_start);
+
+size = load_ramdisk(filename, start, mem_size - start);
+if (size < 0) {
+size = load_image_targphys(filename, start, mem_size - start);
+if (size < 0) {
+error_report("could not load ramdisk '%s'", filename);
+exit(1);
+}
+}
+
+qemu_fdt_setprop_cell(fdt, "/chosen",
+ "linux,initrd-start", start);
+qemu_fdt_setprop_cell(fdt, "/chosen",
+ "linux,initrd-end", start + size);
+
+return start + size;
+}
+
static uint32_t openrisc_load_fdt(Or1ksimState *state, hwaddr load_start,
uint64_t mem_size)
{
@@ -393,6 +420,10 @@ static void openrisc_sim_init(MachineState *machine)
load_addr = openrisc_load_kernel(ram_size, kernel_filename);
if (load_addr > 0) {
+if (machine->initrd_filename) {
+load_addr = openrisc_load_initrd(state, machine->initrd_filename,
+ load_addr, machine->ram_size);
+}
boot_info.fdt_addr = openrisc_load_fdt(state, load_addr,
machine->ram_size);
}
--
2.31.1
Re: [PATCH v2 1/2] hw/ide: split bmdma read and write functions from piix.c
On 2/19/22 02:12, BALATON Zoltan wrote: On Fri, 18 Feb 2022, Liav Albani wrote: This is a preparation before implementing another PCI IDE controller that relies on these functions, so these can be shared between both implementations. Signed-off-by: Liav Albani --- hw/ide/bmdma.c | 84 ++ hw/ide/meson.build | 2 +- hw/ide/piix.c | 51 ++--- include/hw/ide/bmdma.h | 34 + 4 files changed, 122 insertions(+), 49 deletions(-) create mode 100644 hw/ide/bmdma.c create mode 100644 include/hw/ide/bmdma.h diff --git a/hw/ide/bmdma.c b/hw/ide/bmdma.c new file mode 100644 index 00..d12ed730dd --- /dev/null +++ b/hw/ide/bmdma.c @@ -0,0 +1,84 @@ +/* + * QEMU IDE Emulation: Intel PCI Bus master IDE support for PIIX3/4 and ICH6/7. + * + * Copyright (c) 2003 Fabrice Bellard + * Copyright (c) 2006 Openedhand Ltd. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "hw/pci/pci.h" +#include "migration/vmstate.h" +#include "qapi/error.h" +#include "qemu/module.h" +#include "sysemu/block-backend.h" +#include "sysemu/blockdev.h" +#include "sysemu/dma.h" + +#include "hw/ide/bmdma.h" +#include "hw/ide/pci.h" +#include "trace.h" Are you sure you need all these includes? I haven't checked but I think there may be some unneeded ones here. On the other hand I'm not sure putting these in a new file is worth it. There are already some bmdma_* functions in hw/ide/pci.c which are declared in include/hw/ide/pci.h so you could just move these there too then no new file, new header nor changes to meson.build is needed in this patch.. Good question, probably not. I'll try to build without them, adding only what seems necessary to complete the build. Also, I think adding those functions to hw/ide/pci.c is a very good idea as it will make the patch smaller and it also makes total sense to me - Bus Master capabilities only appear on PCI IDE controllers and not on the ISA IDE controller (AFAIK). It will happen in v3 :) + +uint64_t intel_ide_bmdma_read(void *opaque, hwaddr addr, unsigned size) As I said before these aren't intel specific as the same functions also appear in other ide controllers such as via.c so maybe a better name would be bmdma_default_read and bmdma_default_write or somehting similar so these can be also reused by other non-intel devices too. Right, I see now that via.c uses the exact same functions... I'll change it in v3. The names bmdma_default_read and bmdma_default_write seem like a good choice to me. Regards, BALATON Zoltan Thanks for the review!
[PATCH v3 2/6] hw/openrisc/openrisc_sim: Parameterize initialization
Move magic numbers to variables and enums. These will be reused for
upcoming fdt initialization.
Signed-off-by: Stafford Horne
Reviewed-by: Philippe Mathieu-Daudé
---
hw/openrisc/openrisc_sim.c | 42 ++
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 26d2370e60..d12b3e0c5e 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -49,6 +49,29 @@ typedef struct Or1ksimState {
} Or1ksimState;
+enum {
+OR1KSIM_DRAM,
+OR1KSIM_UART,
+OR1KSIM_ETHOC,
+OR1KSIM_OMPIC,
+};
+
+enum {
+OR1KSIM_OMPIC_IRQ = 1,
+OR1KSIM_UART_IRQ = 2,
+OR1KSIM_ETHOC_IRQ = 4,
+};
+
+static const struct MemmapEntry {
+hwaddr base;
+hwaddr size;
+} or1ksim_memmap[] = {
+[OR1KSIM_DRAM] = { 0x, 0 },
+[OR1KSIM_UART] = { 0x9000, 0x100 },
+[OR1KSIM_ETHOC] = { 0x9200, 0x800 },
+[OR1KSIM_OMPIC] = { 0x9800, 16 },
+};
+
static struct openrisc_boot_info {
uint32_t bootstrap_pc;
} boot_info;
@@ -176,21 +199,24 @@ static void openrisc_sim_init(MachineState *machine)
memory_region_add_subregion(get_system_memory(), 0, ram);
if (nd_table[0].used) {
-openrisc_sim_net_init(0x9200, 0x92000400, smp_cpus,
- cpus, 4, nd_table);
+openrisc_sim_net_init(or1ksim_memmap[OR1KSIM_ETHOC].base,
+ or1ksim_memmap[OR1KSIM_ETHOC].base + 0x400,
+ smp_cpus, cpus,
+ OR1KSIM_ETHOC_IRQ, nd_table);
}
if (smp_cpus > 1) {
-openrisc_sim_ompic_init(0x9800, smp_cpus, cpus, 1);
+openrisc_sim_ompic_init(or1ksim_memmap[OR1KSIM_OMPIC].base, smp_cpus,
+cpus, OR1KSIM_OMPIC_IRQ);
-serial_irq = qemu_irq_split(get_cpu_irq(cpus, 0, 2),
-get_cpu_irq(cpus, 1, 2));
+serial_irq = qemu_irq_split(get_cpu_irq(cpus, 0, OR1KSIM_UART_IRQ),
+get_cpu_irq(cpus, 1, OR1KSIM_UART_IRQ));
} else {
-serial_irq = get_cpu_irq(cpus, 0, 2);
+serial_irq = get_cpu_irq(cpus, 0, OR1KSIM_UART_IRQ);
}
-serial_mm_init(get_system_memory(), 0x9000, 0, serial_irq,
- 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
+serial_mm_init(get_system_memory(), or1ksim_memmap[OR1KSIM_UART].base, 0,
+ serial_irq, 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
openrisc_load_kernel(ram_size, kernel_filename);
}
--
2.31.1
[PATCH v3 3/6] hw/openrisc/openrisc_sim: Use IRQ splitter when connecting UART
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);
+}
+serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
+ serial_hd(0), DEVICE_NATIVE_ENDIAN);
+}
+
+
static void openrisc_load_kernel(ram_addr_t ram_size,
const char *kernel_filename)
{
@@ -177,7 +199,6 @@ static void openrisc_sim_init(MachineState *machine)
const char *kernel_filename = machine->kernel_filename;
OpenRISCCPU *cpus[2] = {};
MemoryRegion *ram;
-qemu_irq serial_irq;
int n;
unsigned int smp_cpus = machine->smp.cpus;
@@ -208,15 +229,10 @@ static void openrisc_sim_init(MachineState *machine)
if (smp_cpus > 1) {
openrisc_sim_ompic_init(or1ksim_memmap[OR1KSIM_OMPIC].base, smp_cpus,
cpus, OR1KSIM_OMPIC_IRQ);
-
-serial_irq = qemu_irq_split(get_cpu_irq(cpus, 0, OR1KSIM_UART_IRQ),
-get_cpu_irq(cpus, 1, OR1KSIM_UART_IRQ));
-} else {
-serial_irq = get_cpu_irq(cpus, 0, OR1KSIM_UART_IRQ);
}
-serial_mm_init(get_system_memory(), or1ksim_memmap[OR1KSIM_UART].base, 0,
- serial_irq, 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
+openrisc_sim_serial_init(or1ksim_memmap[OR1KSIM_UART].base, smp_cpus, cpus,
+ OR1KSIM_UART_IRQ);
openrisc_load_kernel(ram_size, kernel_filename);
}
--
2.31.1
[PATCH v3 5/6] hw/openrisc/openrisc_sim: Add automatic device tree generation
Using the device tree means that qemu can now directly tell
the kernel what hardware is configured rather than use having
to maintain and update a separate device tree file.
This patch adds automatic device tree generation support for the
OpenRISC simulator. A device tree is built up based on the state of the
configure openrisc simulator.
This is then dumped to memory and the load address is passed to the
kernel in register r3.
Signed-off-by: Stafford Horne
---
configs/targets/or1k-softmmu.mak | 1 +
hw/openrisc/openrisc_sim.c | 189 ---
2 files changed, 175 insertions(+), 15 deletions(-)
diff --git a/configs/targets/or1k-softmmu.mak b/configs/targets/or1k-softmmu.mak
index 1dfb93e46d..9e1d4a1fb1 100644
--- a/configs/targets/or1k-softmmu.mak
+++ b/configs/targets/or1k-softmmu.mak
@@ -1,2 +1,3 @@
TARGET_ARCH=openrisc
TARGET_WORDS_BIGENDIAN=y
+TARGET_NEED_FDT=y
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 8cfb92bec6..e0e71c0faa 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -29,15 +29,20 @@
#include "net/net.h"
#include "hw/loader.h"
#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "sysemu/device_tree.h"
#include "sysemu/sysemu.h"
#include "hw/sysbus.h"
#include "sysemu/qtest.h"
#include "sysemu/reset.h"
#include "hw/core/split-irq.h"
+#include
+
#define KERNEL_LOAD_ADDR 0x100
#define OR1KSIM_CPUS_MAX 4
+#define OR1KSIM_CLK_MHZ 2000
#define TYPE_OR1KSIM_MACHINE MACHINE_TYPE_NAME("or1k-sim")
#define OR1KSIM_MACHINE(obj) \
@@ -48,6 +53,8 @@ typedef struct Or1ksimState {
MachineState parent_obj;
/*< public >*/
+void *fdt;
+int fdt_size;
} Or1ksimState;
@@ -76,6 +83,7 @@ static const struct MemmapEntry {
static struct openrisc_boot_info {
uint32_t bootstrap_pc;
+uint32_t fdt_addr;
} boot_info;
static void main_cpu_reset(void *opaque)
@@ -86,6 +94,7 @@ static void main_cpu_reset(void *opaque)
cpu_reset(CPU(cpu));
cpu_set_pc(cs, boot_info.bootstrap_pc);
+cpu_set_gpr(&cpu->env, 3, boot_info.fdt_addr);
}
static qemu_irq get_cpu_irq(OpenRISCCPU *cpus[], int cpunum, int irq_pin)
@@ -93,12 +102,77 @@ static qemu_irq get_cpu_irq(OpenRISCCPU *cpus[], int
cpunum, int irq_pin)
return qdev_get_gpio_in_named(DEVICE(cpus[cpunum]), "IRQ", irq_pin);
}
-static void openrisc_sim_net_init(hwaddr base, hwaddr descriptors,
+static void openrisc_create_fdt(Or1ksimState *state,
+const struct MemmapEntry *memmap,
+int num_cpus, uint64_t mem_size,
+const char *cmdline)
+{
+void *fdt;
+int cpu;
+char *nodename;
+int pic_ph;
+
+fdt = state->fdt = create_device_tree(&state->fdt_size);
+if (!fdt) {
+error_report("create_device_tree() failed");
+exit(1);
+}
+
+qemu_fdt_setprop_string(fdt, "/", "compatible", "opencores,or1ksim");
+qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x1);
+qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x1);
+
+nodename = g_strdup_printf("/memory@%" HWADDR_PRIx,
+ memmap[OR1KSIM_DRAM].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cells(fdt, nodename, "reg",
+ memmap[OR1KSIM_DRAM].base, mem_size);
+qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");
+g_free(nodename);
+
+qemu_fdt_add_subnode(fdt, "/cpus");
+qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
+qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
+
+for (cpu = 0; cpu < num_cpus; cpu++) {
+nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_string(fdt, nodename, "compatible",
+"opencores,or1200-rtlsvn481");
+qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu);
+qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
+ OR1KSIM_CLK_MHZ);
+g_free(nodename);
+}
+
+nodename = (char *)"/pic";
+qemu_fdt_add_subnode(fdt, nodename);
+pic_ph = qemu_fdt_alloc_phandle(fdt);
+qemu_fdt_setprop_string(fdt, nodename, "compatible",
+"opencores,or1k-pic-level");
+qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
+qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+qemu_fdt_setprop_cell(fdt, nodename, "phandle", pic_ph);
+
+qemu_fdt_setprop_cell(fdt, "/", "interrupt-parent", pic_ph);
+
+qemu_fdt_add_subnode(fdt, "/chosen");
+if (cmdline) {
+qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
+}
+
+/* Create aliases node for use by devices. */
+qemu_fdt_add_subnode(fdt, "/aliases");
+}
+
+static void openrisc_sim_net_init(Or1ksimState *state, hwaddr base, hwaddr
size,
[PATCH v3 1/6] hw/openrisc/openrisc_sim: Create machine state for or1ksim
This will allow us to attach machine state attributes like
the device tree fdt.
Signed-off-by: Stafford Horne
Reviewed-by: Philippe Mathieu-Daudé
---
hw/openrisc/openrisc_sim.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 73fe383c2d..26d2370e60 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -37,6 +37,18 @@
#define KERNEL_LOAD_ADDR 0x100
+#define TYPE_OR1KSIM_MACHINE MACHINE_TYPE_NAME("or1k-sim")
+#define OR1KSIM_MACHINE(obj) \
+OBJECT_CHECK(Or1ksimState, (obj), TYPE_OR1KSIM_MACHINE)
+
+typedef struct Or1ksimState {
+/*< private >*/
+MachineState parent_obj;
+
+/*< public >*/
+
+} Or1ksimState;
+
static struct openrisc_boot_info {
uint32_t bootstrap_pc;
} boot_info;
@@ -183,8 +195,10 @@ static void openrisc_sim_init(MachineState *machine)
openrisc_load_kernel(ram_size, kernel_filename);
}
-static void openrisc_sim_machine_init(MachineClass *mc)
+static void openrisc_sim_machine_init(ObjectClass *oc, void *data)
{
+MachineClass *mc = MACHINE_CLASS(oc);
+
mc->desc = "or1k simulation";
mc->init = openrisc_sim_init;
mc->max_cpus = 2;
@@ -192,4 +206,16 @@ static void openrisc_sim_machine_init(MachineClass *mc)
mc->default_cpu_type = OPENRISC_CPU_TYPE_NAME("or1200");
}
-DEFINE_MACHINE("or1k-sim", openrisc_sim_machine_init)
+static const TypeInfo or1ksim_machine_typeinfo = {
+.name = TYPE_OR1KSIM_MACHINE,
+.parent = TYPE_MACHINE,
+.class_init = openrisc_sim_machine_init,
+.instance_size = sizeof(Or1ksimState),
+};
+
+static void or1ksim_machine_init_register_types(void)
+{
+type_register_static(&or1ksim_machine_typeinfo);
+}
+
+type_init(or1ksim_machine_init_register_types)
--
2.31.1
[PATCH v3 0/6] OpenRISC Device Tree Generation
Changes since v2: - Fix description to say devicetree "generation" not "support" - Fix up various indentation issues. - Use HWADDR_PRIx string formatting. - Split device tree population out to device init. - Do not load device tree or initrd if we have no kernel. - Added patches to split uart initialization out to it's own function fix 2 CPU limit. Changes since v1: - Fixed typos pointed out by Philippe - Moved usage of machine state to patch 3/4 - added config dependency on FDT This series adds device tree generation for the OpenRISC SIM hardware. The simulator will generate an FDT and pass it to the kernel. For example: qemu-system-or1k -cpu or1200 -M or1k-sim \ -kernel /home/shorne/work/linux/vmlinux \ -initrd /home/shorne/work/linux/initramfs.cpio.gz \ -serial mon:stdio -nographic -gdb tcp::10001 -m 32 Using the linux kernel or1ksim_defconfig we can remove the built-in dts and the kernel will boot as expected. The real benefit here is being able to specify an external initrd which qemu will load into memory and the device tree will tell the kernel where to find it. Tested this by booting single core and SMP kernels with 4x CPUs. Stafford Horne (6): hw/openrisc/openrisc_sim: Create machine state for or1ksim hw/openrisc/openrisc_sim: Parameterize initialization hw/openrisc/openrisc_sim: Use IRQ splitter when connecting UART hw/openrisc/openrisc_sim: Increase max_cpus to 4 hw/openrisc/openrisc_sim: Add automatic device tree generation hw/openrisc/openrisc_sim: Add support for initrd loading configs/targets/or1k-softmmu.mak | 1 + hw/openrisc/openrisc_sim.c | 308 --- 2 files changed, 285 insertions(+), 24 deletions(-) -- 2.31.1
[PATCH v3 4/6] hw/openrisc/openrisc_sim: Increase max_cpus to 4
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(-)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 5bfbac00f8..8cfb92bec6 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -37,6 +37,8 @@
#define KERNEL_LOAD_ADDR 0x100
+#define OR1KSIM_CPUS_MAX 4
+
#define TYPE_OR1KSIM_MACHINE MACHINE_TYPE_NAME("or1k-sim")
#define OR1KSIM_MACHINE(obj) \
OBJECT_CHECK(Or1ksimState, (obj), TYPE_OR1KSIM_MACHINE)
@@ -197,12 +199,12 @@ static void openrisc_sim_init(MachineState *machine)
{
ram_addr_t ram_size = machine->ram_size;
const char *kernel_filename = machine->kernel_filename;
-OpenRISCCPU *cpus[2] = {};
+OpenRISCCPU *cpus[OR1KSIM_CPUS_MAX] = {};
MemoryRegion *ram;
int n;
unsigned int smp_cpus = machine->smp.cpus;
-assert(smp_cpus >= 1 && smp_cpus <= 2);
+assert(smp_cpus >= 1 && smp_cpus <= OR1KSIM_CPUS_MAX);
for (n = 0; n < smp_cpus; n++) {
cpus[n] = OPENRISC_CPU(cpu_create(machine->cpu_type));
if (cpus[n] == NULL) {
@@ -243,7 +245,7 @@ static void openrisc_sim_machine_init(ObjectClass *oc, void
*data)
mc->desc = "or1k simulation";
mc->init = openrisc_sim_init;
-mc->max_cpus = 2;
+mc->max_cpus = OR1KSIM_CPUS_MAX;
mc->is_default = true;
mc->default_cpu_type = OPENRISC_CPU_TYPE_NAME("or1200");
}
--
2.31.1
[PATCH v2] hvf: Fix OOB write in RDTSCP instruction decode
A guest could craft a specific stream of instructions that will have QEMU
write 0xF9 to inappropriate locations in memory. Add additional asserts
to check for this. Generate a #UD if there are more than 14 prefix bytes.
Found by Julian Stecklina
Signed-off-by: Cameron Esfahani
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/hvf/x86_decode.c | 12 ++--
target/i386/hvf/x86hvf.c | 8
target/i386/hvf/x86hvf.h | 1 +
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index 062713b1a4..5d051252b4 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -24,8 +24,10 @@
#include "vmx.h"
#include "x86_mmu.h"
#include "x86_descr.h"
+#include "x86hvf.h"
#define OPCODE_ESCAPE 0xf
+#define X86_MAX_INSN_PREFIX_LENGTH (14)
static void decode_invalid(CPUX86State *env, struct x86_decode *decode)
{
@@ -541,7 +543,8 @@ static void decode_lidtgroup(CPUX86State *env, struct
x86_decode *decode)
};
decode->cmd = group[decode->modrm.reg];
if (0xf9 == decode->modrm.modrm) {
-decode->opcode[decode->len++] = decode->modrm.modrm;
+VM_PANIC_ON(decode->opcode_len >= sizeof(decode->opcode));
+decode->opcode[decode->opcode_len++] = decode->modrm.modrm;
decode->cmd = X86_DECODE_CMD_RDTSCP;
}
}
@@ -1847,7 +1850,8 @@ void calc_modrm_operand(CPUX86State *env, struct
x86_decode *decode,
static void decode_prefix(CPUX86State *env, struct x86_decode *decode)
{
-while (1) {
+/* At most X86_MAX_INSN_PREFIX_LENGTH prefix bytes. */
+for (int i = 0; i < X86_MAX_INSN_PREFIX_LENGTH; i++) {
/*
* REX prefix must come after legacy prefixes.
* REX before legacy is ignored.
@@ -1892,6 +1896,8 @@ static void decode_prefix(CPUX86State *env, struct
x86_decode *decode)
return;
}
}
+/* Too many prefixes! Generate #UD. */
+hvf_inject_ud(env);
}
void set_addressing_size(CPUX86State *env, struct x86_decode *decode)
@@ -2090,11 +2096,13 @@ static void decode_opcodes(CPUX86State *env, struct
x86_decode *decode)
uint8_t opcode;
opcode = decode_byte(env, decode);
+VM_PANIC_ON(decode->opcode_len >= sizeof(decode->opcode));
decode->opcode[decode->opcode_len++] = opcode;
if (opcode != OPCODE_ESCAPE) {
decode_opcode_1(env, decode, opcode);
} else {
opcode = decode_byte(env, decode);
+VM_PANIC_ON(decode->opcode_len >= sizeof(decode->opcode));
decode->opcode[decode->opcode_len++] = opcode;
decode_opcode_2(env, decode, opcode);
}
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index 05ec1bddc4..a805c125d9 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -425,6 +425,14 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
& (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR));
}
+void hvf_inject_ud(CPUX86State *env)
+{
+env->exception_nr = EXCP06_ILLOP;
+env->exception_injected = 1;
+env->has_error_code = false;
+env->error_code = 0;
+}
+
int hvf_process_events(CPUState *cpu_state)
{
X86CPU *cpu = X86_CPU(cpu_state);
diff --git a/target/i386/hvf/x86hvf.h b/target/i386/hvf/x86hvf.h
index 99ed8d608d..ef472a32f9 100644
--- a/target/i386/hvf/x86hvf.h
+++ b/target/i386/hvf/x86hvf.h
@@ -22,6 +22,7 @@
int hvf_process_events(CPUState *);
bool hvf_inject_interrupts(CPUState *);
+void hvf_inject_ud(CPUX86State *);
void hvf_set_segment(struct CPUState *cpu, struct vmx_segment *vmx_seg,
SegmentCache *qseg, bool is_tr);
void hvf_get_segment(SegmentCache *qseg, struct vmx_segment *vmx_seg);
--
2.32.0 (Apple Git-132)
Re: [PATCH v2 2/2] hw/ide: add ich6 ide controller device emulation
On Fri, 18 Feb 2022, Liav Albani wrote:
This type of IDE controller has support for relocating the IO ports and
doesn't use IRQ 14 and 15 but one allocated PCI IRQ for the controller.
There's no x86 chipset in QEMU that will try to attach this device by
default. It is considered a legacy-free device in the aspect of PCI bus
resource management as the guest OS can relocate the IO ports as it sees
fit to its needs. However, this is still a legacy device that belongs to
chipsets from late 2000s.
Signed-off-by: Liav Albani
---
hw/i386/Kconfig | 2 +
hw/ide/Kconfig | 5 +
hw/ide/ich6.c| 204 +++
hw/ide/meson.build | 1 +
include/hw/ide/pci.h | 1 +
include/hw/pci/pci_ids.h | 1 +
6 files changed, 214 insertions(+)
create mode 100644 hw/ide/ich6.c
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index d22ac4a4b9..a18de2d962 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -75,6 +75,7 @@ config I440FX
select PCI_I440FX
select PIIX3
select IDE_PIIX
+select IDE_ICH6
select DIMM
select SMBIOS
select FW_CFG_DMA
@@ -101,6 +102,7 @@ config Q35
select PCI_EXPRESS_Q35
select LPC_ICH9
select AHCI_ICH9
+select IDE_ICH6
select DIMM
select SMBIOS
select FW_CFG_DMA
diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
index dd85fa3619..63304325a5 100644
--- a/hw/ide/Kconfig
+++ b/hw/ide/Kconfig
@@ -38,6 +38,11 @@ config IDE_VIA
select IDE_PCI
select IDE_QDEV
+config IDE_ICH6
+bool
+select IDE_PCI
+select IDE_QDEV
+
config MICRODRIVE
bool
select IDE_QDEV
diff --git a/hw/ide/ich6.c b/hw/ide/ich6.c
new file mode 100644
index 00..8f46d3fce2
--- /dev/null
+++ b/hw/ide/ich6.c
@@ -0,0 +1,204 @@
+/*
+ * QEMU IDE Emulation: PCI ICH6/ICH7 IDE support.
This is a small thing, but if these two are the same maybe keeping this
comment but using the ich7 name everywhere else would make it less likely
to get it confused with ich9. I mean ich6 and ich9 is easily confused,
while ich7 is clearly distinct. But maybe it's just me, calling it ich6 is
also correct and can be preferred by someone else.
+ *
+ * Copyright (c) 2022 Liav Albani
+ *
+ * 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/pci/pci.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/dma.h"
+
+#include "hw/ide/pci.h"
+#include "hw/ide/bmdma.h"
+#include "trace.h"
+
+static const MemoryRegionOps ich6_bmdma_ops = {
+.read = intel_ide_bmdma_read,
+.write = intel_ide_bmdma_write,
+};
+
+static void bmdma_setup_bar(PCIIDEState *d)
+{
+int i;
+
+memory_region_init(&d->bmdma_bar, OBJECT(d), "ich6-bmdma-container", 16);
+for (i = 0; i < 2; i++) {
+BMDMAState *bm = &d->bmdma[i];
+
+memory_region_init_io(&bm->extra_io, OBJECT(d), &ich6_bmdma_ops, bm,
+ "ich6-bmdma", 4);
+memory_region_add_subregion(&d->bmdma_bar, i * 8, &bm->extra_io);
+memory_region_init_io(&bm->addr_ioport, OBJECT(d),
+ &bmdma_addr_ioport_ops, bm, "bmdma", 4);
+memory_region_add_subregion(&d->bmdma_bar, i * 8 + 4,
&bm->addr_ioport);
+}
+}
+
+static void ich6_pci_config_write(PCIDevice *d, uint32_t addr, uint32_t val,
+int l)
+{
+uint32_t i;
+
+pci_default_write_config(d, addr, val, l);
+
+for (i = addr; i < addr + l; i++) {
+switch (i) {
+case 0x40:
+pci_default_write_config(d, i, 0x8000, 2);
+continue;
+case 0x42:
+pci_default_write_config(d, i, 0x8000, 2);
+continue;
+}
+}
I'm not sure what this tries to do but this for cycle looks suspicious
here. It's also only calls pci_default_write_config() so why didn'
[PATCH v5 06/12] target/riscv: Add support for hpmcounters/hpmevents
From: Atish Patra
With SBI PMU extension, user can use any of the available hpmcounters to
track any perf events based on the value written to mhpmevent csr.
Add read/write functionality for these csrs.
Reviewed-by: Bin Meng
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.h | 11 +
target/riscv/csr.c | 466 +++--
target/riscv/machine.c | 3 +
3 files changed, 328 insertions(+), 152 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ea3862ccbf5c..cce5c3538c89 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -109,6 +109,8 @@ typedef struct CPURISCVState CPURISCVState;
#endif
#define RV_VLEN_MAX 1024
+#define RV_MAX_MHPMEVENTS 29
+#define RV_MAX_MHPMCOUNTERS 32
FIELD(VTYPE, VLMUL, 0, 3)
FIELD(VTYPE, VSEW, 3, 3)
@@ -261,6 +263,15 @@ struct CPURISCVState {
target_ulong mcountinhibit;
+/* PMU counter configured values */
+target_ulong mhpmcounter_val[RV_MAX_MHPMCOUNTERS];
+
+/* for RV32 */
+target_ulong mhpmcounterh_val[RV_MAX_MHPMCOUNTERS];
+
+/* PMU event selector configured values */
+target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
+
target_ulong sscratch;
target_ulong mscratch;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 2283ff33a5d7..dbb723a3307b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -84,6 +84,15 @@ static RISCVException mctr(CPURISCVState *env, int csrno)
return RISCV_EXCP_NONE;
}
+static RISCVException mctr32(CPURISCVState *env, int csrno)
+{
+if (riscv_cpu_mxl(env) != MXL_RV32) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return mctr(env, csrno);
+}
+
static RISCVException ctr(CPURISCVState *env, int csrno)
{
#if !defined(CONFIG_USER_ONLY)
@@ -560,6 +569,72 @@ static RISCVException read_instreth(CPURISCVState *env,
int csrno,
return RISCV_EXCP_NONE;
}
+static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong *val)
+{
+int evt_index = csrno - CSR_MHPMEVENT3;
+
+*val = env->mhpmevent_val[evt_index];
+
+return RISCV_EXCP_NONE;
+}
+
+static int write_mhpmevent(CPURISCVState *env, int csrno, target_ulong val)
+{
+int evt_index = csrno - CSR_MHPMEVENT3;
+
+env->mhpmevent_val[evt_index] = val;
+
+return RISCV_EXCP_NONE;
+}
+
+static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val)
+{
+int ctr_index = csrno - CSR_MHPMCOUNTER3 + 3;
+
+env->mhpmcounter_val[ctr_index] = val;
+
+return RISCV_EXCP_NONE;
+}
+
+static int write_mhpmcounterh(CPURISCVState *env, int csrno, target_ulong val)
+{
+int ctr_index = csrno - CSR_MHPMCOUNTER3H + 3;
+
+env->mhpmcounterh_val[ctr_index] = val;
+
+return RISCV_EXCP_NONE;
+}
+
+static int read_hpmcounter(CPURISCVState *env, int csrno, target_ulong *val)
+{
+int ctr_index;
+
+if (env->priv == PRV_M) {
+ctr_index = csrno - CSR_MHPMCOUNTER3 + 3;
+} else {
+ctr_index = csrno - CSR_HPMCOUNTER3 + 3;
+}
+*val = env->mhpmcounter_val[ctr_index];
+
+return RISCV_EXCP_NONE;
+}
+
+static int read_hpmcounterh(CPURISCVState *env, int csrno, target_ulong *val)
+{
+int ctr_index;
+
+if (env->priv == PRV_M) {
+ctr_index = csrno - CSR_MHPMCOUNTER3H + 3;
+} else {
+ctr_index = csrno - CSR_HPMCOUNTER3H + 3;
+}
+
+*val = env->mhpmcounterh_val[ctr_index];
+
+return RISCV_EXCP_NONE;
+}
+
+
#if defined(CONFIG_USER_ONLY)
static RISCVException read_time(CPURISCVState *env, int csrno,
target_ulong *val)
@@ -3515,157 +3590,244 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_SPMBASE] ={ "spmbase", pointer_masking, read_spmbase,
write_spmbase },
/* Performance Counters */
-[CSR_HPMCOUNTER3]= { "hpmcounter3",ctr,read_zero },
-[CSR_HPMCOUNTER4]= { "hpmcounter4",ctr,read_zero },
-[CSR_HPMCOUNTER5]= { "hpmcounter5",ctr,read_zero },
-[CSR_HPMCOUNTER6]= { "hpmcounter6",ctr,read_zero },
-[CSR_HPMCOUNTER7]= { "hpmcounter7",ctr,read_zero },
-[CSR_HPMCOUNTER8]= { "hpmcounter8",ctr,read_zero },
-[CSR_HPMCOUNTER9]= { "hpmcounter9",ctr,read_zero },
-[CSR_HPMCOUNTER10] = { "hpmcounter10", ctr,read_zero },
-[CSR_HPMCOUNTER11] = { "hpmcounter11", ctr,read_zero },
-[CSR_HPMCOUNTER12] = { "hpmcounter12", ctr,read_zero },
-[CSR_HPMCOUNTER13] = { "hpmcounter13", ctr,read_zero },
-[CSR_HPMCOUNTER14] = { "hpmcounter14", ctr,read_zero },
-[CSR_HPMCOUNTER15] = { "hpmcounter15", ctr,read_zero },
-[CSR_HPMCOUNTER16] = { "hpmcounter16", ctr,read_zero },
-[CSR_HPMCOUNTER17] = { "hpmcounter17", ctr,read_zero },
-[CSR_HPMCOUNTER18] = { "hpmcounter18", ctr,read_zero },
-[CSR_HPMCOUNTER19] = { "hpmcounter19", ctr,read_zero },
-[CSR_HPMCOUNTER20] = { "hpmcou
[PATCH v5 12/12] target/riscv: Update the privilege field for sscofpmf CSRs
The sscofpmf extension was ratified as a part of priv spec v1.12.
Mark the csr_ops accordingly.
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 90 ++
1 file changed, 60 insertions(+), 30 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 54966a770672..0407ff12b445 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3786,63 +3786,92 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
write_mhpmevent },
[CSR_MHPMEVENT3H]= { "mhpmevent3h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT4H]= { "mhpmevent4h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT5H]= { "mhpmevent5h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT6H]= { "mhpmevent6h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT7H]= { "mhpmevent7h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT8H]= { "mhpmevent8h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT9H]= { "mhpmevent9h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT10H] = { "mhpmevent10h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT11H] = { "mhpmevent11h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT12H] = { "mhpmevent12h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT13H] = { "mhpmevent13h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT14H] = { "mhpmevent14h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT15H] = { "mhpmevent15h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT16H] = { "mhpmevent16h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh},
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MHPMEVENT17H] = { "mhpmevent17h",sscofpmf, read_mhpmeventh,
-
[PATCH v5 11/12] hw/riscv: virt: Add PMU DT node to the device tree
Qemu virt machine can support few cache events and cycle/instret counters.
It also supports counter overflow for these events.
Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
capabilities. There are some dummy nodes added for testing as well.
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
hw/riscv/virt.c| 28 +++
target/riscv/cpu.c | 1 +
target/riscv/pmu.c | 57 ++
target/riscv/pmu.h | 1 +
4 files changed, 87 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 7d5f1e58c983..6288e436aa73 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -28,6 +28,7 @@
#include "hw/qdev-properties.h"
#include "hw/char/serial.h"
#include "target/riscv/cpu.h"
+#include "target/riscv/pmu.h"
#include "hw/riscv/riscv_hart.h"
#include "hw/riscv/virt.h"
#include "hw/riscv/boot.h"
@@ -687,6 +688,32 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
aplic_phandles[socket] = aplic_s_phandle;
}
+static void create_fdt_socket_pmu(RISCVVirtState *s,
+ int socket, uint32_t *phandle,
+ uint32_t *intc_phandles)
+{
+int cpu;
+char *pmu_name;
+uint32_t *pmu_cells;
+MachineState *mc = MACHINE(s);
+RISCVCPU hart = s->soc[socket].harts[0];
+
+pmu_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
+
+for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
+pmu_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
+pmu_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_PMU_OVF);
+}
+
+pmu_name = g_strdup_printf("/soc/pmu");
+qemu_fdt_add_subnode(mc->fdt, pmu_name);
+qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible", "riscv,pmu");
+riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
+
+g_free(pmu_name);
+g_free(pmu_cells);
+}
+
static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
bool is_32_bit, uint32_t *phandle,
uint32_t *irq_mmio_phandle,
@@ -732,6 +759,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const
MemMapEntry *memmap,
&intc_phandles[phandle_pos]);
}
}
+create_fdt_socket_pmu(s, socket, phandle, intc_phandles);
}
if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 677210bc6d94..00c385009d67 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -910,6 +910,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char
*isa_str, int max_str_len)
{ "svpbmt", cpu->cfg.ext_svpbmt },
{ "svinval", cpu->cfg.ext_svinval },
{ "svnapot", cpu->cfg.ext_svnapot },
+{ "sscofpmf", cpu->cfg.ext_sscofpmf },
};
for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index 5b212d2eb630..6e470a1d5f66 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -19,11 +19,68 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "pmu.h"
+#include "sysemu/device_tree.h"
#define RISCV_TIMEBASE_FREQ 10 /* 1Ghz */
#define MAKE_32BIT_MASK(shift, length) \
(((uint32_t)(~0UL) >> (32 - (length))) << (shift))
+/**
+ * To keep it simple, any event can be mapped to any programmable counters in
+ * QEMU. The generic cycle & instruction count events can also be monitored
+ * using programmable counters. In that case, mcycle & minstret must continue
+ * to provide the correct value as well. Heterogeneous PMU per hart is not
+ * supported yet. Thus, number of counters are same across all harts.
+ */
+void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
+{
+uint32_t fdt_event_ctr_map[20] = {};
+uint32_t cmask;
+
+/* All the programmable counters can map to any event */
+cmask = MAKE_32BIT_MASK(3, num_ctrs);
+
+ /**
+* The event encoding is specified in the SBI specification
+* Event idx is a 20bits wide number encoded as follows:
+* event_idx[19:16] = type
+* event_idx[15:0] = code
+* The code field in cache events are encoded as follows:
+* event_idx.code[15:3] = cache_id
+* event_idx.code[2:1] = op_id
+* event_idx.code[0:0] = result_id
+*/
+
+ /* SBI_PMU_HW_CPU_CYCLES: 0x01 : type(0x00) */
+ fdt_event_ctr_map[0] = cpu_to_be32(0x0001);
+ fdt_event_ctr_map[1] = cpu_to_be32(0x0001);
+ fdt_event_ctr_map[2] = cpu_to_be32(cmask | 1 << 0);
+
+ /* SBI_PMU_HW_INSTRUCTIONS: 0x02 : type(0x00) */
+ fdt_event_ctr_map[3] = cpu_to_be32(0x0002);
+ fdt_event_ctr_map[4] = cpu_to_be32(0x0002);
+ fdt_event_ctr_map[5] = cpu_to_be32(cmask | 1 << 2);
+
+ /* SBI_PMU_HW_CACHE_DTLB : 0x03 READ : 0x00 MISS : 0x00 type(0x01) */
+ fdt_event_ctr_map[6] = cpu_to_be32(0x00010019);
+ fdt_event_ctr_map[7] = cpu_to_be32(0x00010019);
+ fdt_event_ctr_map[8] = cpu_to_be32(c
[PATCH v5 09/12] target/riscv: Simplify counter predicate function
All the hpmcounters and the fixed counters (CY, IR, TM) can be represented
as a unified counter. Thus, the predicate function doesn't need handle each
case separately.
Simplify the predicate function so that we just handle things differently
between RV32/RV64 and S/HS mode.
Reviewed-by: Bin Meng
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 111 -
1 file changed, 10 insertions(+), 101 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0071b13bc50f..54966a770672 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -113,6 +113,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
CPUState *cs = env_cpu(env);
RISCVCPU *cpu = RISCV_CPU(cs);
int ctr_index;
+target_ulong ctr_mask;
int base_csrno = CSR_CYCLE;
bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
@@ -121,122 +122,30 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
base_csrno += 0x80;
}
ctr_index = csrno - base_csrno;
+ctr_mask = BIT(ctr_index);
if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) ||
(csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) {
goto skip_ext_pmu_check;
}
-if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & BIT(ctr_index {
+if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & ctr_mask))) {
/* No counter is enabled in PMU or the counter is out of range */
return RISCV_EXCP_ILLEGAL_INST;
}
skip_ext_pmu_check:
-if (env->priv == PRV_S) {
-switch (csrno) {
-case CSR_CYCLE:
-if (!get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_TIME:
-if (!get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_INSTRET:
-if (!get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
-if (!get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-}
-if (rv32) {
-switch (csrno) {
-case CSR_CYCLEH:
-if (!get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_TIMEH:
-if (!get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_INSTRETH:
-if (!get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
-if (!get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-}
-}
+if ((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) {
+return RISCV_EXCP_ILLEGAL_INST;
}
if (riscv_cpu_virt_enabled(env)) {
-switch (csrno) {
-case CSR_CYCLE:
-if (!get_field(env->hcounteren, COUNTEREN_CY) &&
-get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_TIME:
-if (!get_field(env->hcounteren, COUNTEREN_TM) &&
-get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_INSTRET:
-if (!get_field(env->hcounteren, COUNTEREN_IR) &&
-get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
-if (!get_field(env->hcounteren, 1 << ctr_index) &&
- get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-}
-if (rv32) {
-switch (csrno) {
-case CSR_CYCLEH:
-if (!get_field(env->hcounteren, COUNTEREN_CY) &&
-get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_TIMEH:
-if (!get_field(env->hcounteren, COUNTEREN_TM) &&
-get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_INSTR
[PATCH v5 04/12] target/riscv: pmu: Make number of counters configurable
The RISC-V privilege specification provides flexibility to implement
any number of counters from 29 programmable counters. However, the QEMU
implements all the counters.
Make it configurable through pmu config parameter which now will indicate
how many programmable counters should be implemented by the cpu.
Reviewed-by: Bin Meng
Reviewed-by: Alistair Francis
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.c | 2 +-
target/riscv/cpu.h | 2 +-
target/riscv/csr.c | 96 ++
3 files changed, 65 insertions(+), 35 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 173b9d3c5d3e..02e089710a7e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -768,7 +768,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
-DEFINE_PROP_BOOL("pmu", RISCVCPU, cfg.ext_pmu, true),
+DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2dc491887f24..f6b994a74ed9 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -365,7 +365,6 @@ struct RISCVCPUConfig {
bool ext_zbb;
bool ext_zbc;
bool ext_zbs;
-bool ext_pmu;
bool ext_ifencei;
bool ext_icsr;
bool ext_svinval;
@@ -379,6 +378,7 @@ struct RISCVCPUConfig {
/* Vendor-specific custom extensions */
bool ext_XVentanaCondOps;
+uint8_t pmu_num;
char *priv_spec;
char *user_spec;
char *bext_spec;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 2c3bba81c8af..d69bd2d88454 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -62,15 +62,45 @@ static RISCVException vs(CPURISCVState *env, int csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
+static RISCVException mctr(CPURISCVState *env, int csrno)
+{
+#if !defined(CONFIG_USER_ONLY)
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+int ctr_index;
+int base_csrno = CSR_MHPMCOUNTER3;
+
+if ((riscv_cpu_mxl(env) == MXL_RV32) && csrno >= CSR_MCYCLEH) {
+/* Offset for RV32 mhpmcounternh counters */
+base_csrno += 0x80;
+}
+ctr_index = csrno - base_csrno;
+if (!cpu->cfg.pmu_num || ctr_index >= cpu->cfg.pmu_num) {
+/* The PMU is not enabled or counter is out of range*/
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+#endif
+return RISCV_EXCP_NONE;
+}
+
static RISCVException ctr(CPURISCVState *env, int csrno)
{
#if !defined(CONFIG_USER_ONLY)
CPUState *cs = env_cpu(env);
RISCVCPU *cpu = RISCV_CPU(cs);
int ctr_index;
+int base_csrno = CSR_HPMCOUNTER3;
+bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
+
+if (rv32 && csrno >= CSR_CYCLEH) {
+/* Offset for RV32 hpmcounternh counters */
+base_csrno += 0x80;
+}
+ctr_index = csrno - base_csrno;
-if (!cpu->cfg.ext_pmu) {
-/* The PMU extension is not enabled */
+if (!cpu->cfg.pmu_num || ctr_index >= (cpu->cfg.pmu_num)) {
+/* No counter is enabled in PMU or the counter is out of range */
return RISCV_EXCP_ILLEGAL_INST;
}
@@ -98,7 +128,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
}
break;
}
-if (riscv_cpu_is_32bit(env)) {
+if (rv32) {
switch (csrno) {
case CSR_CYCLEH:
if (!get_field(env->mcounteren, COUNTEREN_CY)) {
@@ -153,7 +183,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
}
break;
}
-if (riscv_cpu_mxl(env) == MXL_RV32) {
+if (rv32) {
switch (csrno) {
case CSR_CYCLEH:
if (!get_field(env->hcounteren, COUNTEREN_CY) &&
@@ -3493,35 +3523,35 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_HPMCOUNTER30] = { "hpmcounter30", ctr,read_zero },
[CSR_HPMCOUNTER31] = { "hpmcounter31", ctr,read_zero },
-[CSR_MHPMCOUNTER3] = { "mhpmcounter3", any,read_zero },
-[CSR_MHPMCOUNTER4] = { "mhpmcounter4", any,read_zero },
-[CSR_MHPMCOUNTER5] = { "mhpmcounter5", any,read_zero },
-[CSR_MHPMCOUNTER6] = { "mhpmcounter6", any,read_zero },
-[CSR_MHPMCOUNTER7] = { "mhpmcounter7", any,read_zero },
-[CSR_MHPMCOUNTER8] = { "mhpmcounter8", any,read_zero },
-[CSR_MHPMCOUNTER9] = { "mhpmcounter9", any,read_zero },
-[CSR_MHPMCOUNTER10] = { "mhpmcounter10", any,read_zero },
-[CSR_MHPMCOUNTER11] = { "mhpmcounter11", any,read_zero },
-[CSR_MHPMCOUNTER12] = { "mhpmcounter12", any,read_zero },
-[CSR_MHPMCO
[PATCH v5 10/12] target/riscv: Add few cache related PMU events
From: Atish Patra
Qemu can monitor the following cache related PMU events through
tlb_fill functions.
1. DTLB load/store miss
3. ITLB prefetch miss
Increment the PMU counter in tlb_fill function.
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu_helper.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 746335bfd6b9..094d41ba07f7 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -21,10 +21,13 @@
#include "qemu/log.h"
#include "qemu/main-loop.h"
#include "cpu.h"
+#include "pmu.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "trace.h"
#include "semihosting/common-semi.h"
+#include "cpu.h"
+#include "cpu_bits.h"
int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
{
@@ -1174,6 +1177,28 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr
addr,
riscv_raise_exception(env, cs->exception_index, retaddr);
}
+
+static void pmu_tlb_fill_incr_ctr(RISCVCPU *cpu, MMUAccessType access_type)
+{
+enum riscv_pmu_event_idx pmu_event_type;
+
+switch (access_type) {
+case MMU_INST_FETCH:
+pmu_event_type = RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS;
+break;
+case MMU_DATA_LOAD:
+pmu_event_type = RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS;
+break;
+case MMU_DATA_STORE:
+pmu_event_type = RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS;
+break;
+default:
+return;
+}
+
+riscv_pmu_incr_ctr(cpu, pmu_event_type);
+}
+
bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr)
@@ -1270,6 +1295,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
}
}
} else {
+pmu_tlb_fill_incr_ctr(cpu, access_type);
/* Single stage lookup */
ret = get_physical_address(env, &pa, &prot, address, NULL,
access_type, mmu_idx, true, false, false);
--
2.30.2
[PATCH v5 01/12] target/riscv: Fix PMU CSR predicate function
From: Atish Patra
The predicate function calculates the counter index incorrectly for
hpmcounterx. Fix the counter index to reflect correct CSR number.
Fixes: e39a8320b088 ("target/riscv: Support the Virtual Instruction fault")
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index b16881615997..3799ee850087 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -94,8 +94,9 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
}
break;
case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
-if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3)) &&
-get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3))) {
+ctr_index = csrno - CSR_CYCLE;
+if (!get_field(env->hcounteren, 1 << ctr_index) &&
+ get_field(env->mcounteren, 1 << ctr_index)) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
break;
@@ -121,8 +122,9 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
}
break;
case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
-if (!get_field(env->hcounteren, 1 << (csrno -
CSR_HPMCOUNTER3H)) &&
-get_field(env->mcounteren, 1 << (csrno -
CSR_HPMCOUNTER3H))) {
+ctr_index = csrno - CSR_CYCLEH;
+if (!get_field(env->hcounteren, 1 << ctr_index) &&
+ get_field(env->mcounteren, 1 << ctr_index)) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
break;
--
2.30.2
[PATCH v5 07/12] target/riscv: Support mcycle/minstret write operation
From: Atish Patra
mcycle/minstret are actually WARL registers and can be written with any
given value. With SBI PMU extension, it will be used to store a initial
value provided from supervisor OS. The Qemu also need prohibit the counter
increment if mcountinhibit is set.
Support mcycle/minstret through generic counter infrastructure.
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.h | 23 +--
target/riscv/csr.c | 145 +++
target/riscv/machine.c | 25 ++-
target/riscv/meson.build | 1 +
target/riscv/pmu.c | 32 +
target/riscv/pmu.h | 28
6 files changed, 201 insertions(+), 53 deletions(-)
create mode 100644 target/riscv/pmu.c
create mode 100644 target/riscv/pmu.h
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index cce5c3538c89..68522acda4d2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -109,7 +109,7 @@ typedef struct CPURISCVState CPURISCVState;
#endif
#define RV_VLEN_MAX 1024
-#define RV_MAX_MHPMEVENTS 29
+#define RV_MAX_MHPMEVENTS 32
#define RV_MAX_MHPMCOUNTERS 32
FIELD(VTYPE, VLMUL, 0, 3)
@@ -119,6 +119,18 @@ FIELD(VTYPE, VMA, 7, 1)
FIELD(VTYPE, VEDIV, 8, 2)
FIELD(VTYPE, RESERVED, 10, sizeof(target_ulong) * 8 - 11)
+typedef struct PMUCTRState {
+/* Current value of a counter */
+target_ulong mhpmcounter_val;
+/* Current value of a counter in RV32*/
+target_ulong mhpmcounterh_val;
+/* Snapshot values of counter */
+target_ulong mhpmcounter_prev;
+/* Snapshort value of a counter in RV32 */
+target_ulong mhpmcounterh_prev;
+bool started;
+} PMUCTRState;
+
struct CPURISCVState {
target_ulong gpr[32];
target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
@@ -263,13 +275,10 @@ struct CPURISCVState {
target_ulong mcountinhibit;
-/* PMU counter configured values */
-target_ulong mhpmcounter_val[RV_MAX_MHPMCOUNTERS];
-
-/* for RV32 */
-target_ulong mhpmcounterh_val[RV_MAX_MHPMCOUNTERS];
+/* PMU counter state */
+PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
-/* PMU event selector configured values */
+/* PMU event selector configured values. First three are unused*/
target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
target_ulong sscratch;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index dbb723a3307b..dc4d258205b3 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -20,6 +20,7 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
#include "cpu.h"
+#include "pmu.h"
#include "qemu/main-loop.h"
#include "exec/exec-all.h"
@@ -539,39 +540,33 @@ static int write_vcsr(CPURISCVState *env, int csrno,
target_ulong val)
}
/* User Timers and Counters */
-static RISCVException read_instret(CPURISCVState *env, int csrno,
- target_ulong *val)
+static target_ulong get_icount_ticks(bool rv32)
{
+int64_t val;
+target_ulong result;
+
#if !defined(CONFIG_USER_ONLY)
if (icount_enabled()) {
-*val = icount_get();
+val = icount_get();
} else {
-*val = cpu_get_host_ticks();
+val = cpu_get_host_ticks();
}
#else
-*val = cpu_get_host_ticks();
+val = cpu_get_host_ticks();
#endif
-return RISCV_EXCP_NONE;
-}
-static RISCVException read_instreth(CPURISCVState *env, int csrno,
-target_ulong *val)
-{
-#if !defined(CONFIG_USER_ONLY)
-if (icount_enabled()) {
-*val = icount_get() >> 32;
+if (rv32) {
+result = val >> 32;
} else {
-*val = cpu_get_host_ticks() >> 32;
+result = val;
}
-#else
-*val = cpu_get_host_ticks() >> 32;
-#endif
-return RISCV_EXCP_NONE;
+
+return result;
}
static int read_mhpmevent(CPURISCVState *env, int csrno, target_ulong *val)
{
-int evt_index = csrno - CSR_MHPMEVENT3;
+int evt_index = csrno - CSR_MCOUNTINHIBIT;
*val = env->mhpmevent_val[evt_index];
@@ -580,7 +575,7 @@ static int read_mhpmevent(CPURISCVState *env, int csrno,
target_ulong *val)
static int write_mhpmevent(CPURISCVState *env, int csrno, target_ulong val)
{
-int evt_index = csrno - CSR_MHPMEVENT3;
+int evt_index = csrno - CSR_MCOUNTINHIBIT;
env->mhpmevent_val[evt_index] = val;
@@ -589,52 +584,102 @@ static int write_mhpmevent(CPURISCVState *env, int
csrno, target_ulong val)
static int write_mhpmcounter(CPURISCVState *env, int csrno, target_ulong val)
{
-int ctr_index = csrno - CSR_MHPMCOUNTER3 + 3;
+int ctr_idx = csrno - CSR_MCYCLE;
+PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
-env->mhpmcounter_val[ctr_index] = val;
+counter->mhpmcounter_val = val;
+if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
+riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
+counter->mhpmcounter_prev = get_icount_ticks(false);
+} else {
+/* Other counters can keep incrementing from the given v
[PATCH v5 08/12] target/riscv: Add sscofpmf extension support
The Sscofpmf ('Ss' for Privileged arch and Supervisor-level extensions,
and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension allows the perf to handle overflow interrupts and filtering
support. This patch provides a framework for programmable
counters to leverage the extension. As the extension doesn't have any
provision for the overflow bit for fixed counters, the fixed events
can also be monitoring using programmable counters. The underlying
counters for cycle and instruction counters are always running. Thus,
a separate timer device is programmed to handle the overflow.
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.c | 12 ++
target/riscv/cpu.h | 25 +++
target/riscv/cpu_bits.h | 55 +++
target/riscv/csr.c | 159 --
target/riscv/pmu.c | 346 +++-
target/riscv/pmu.h | 8 +
6 files changed, 593 insertions(+), 12 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 02e089710a7e..677210bc6d94 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -22,6 +22,7 @@
#include "qemu/ctype.h"
#include "qemu/log.h"
#include "cpu.h"
+#include "pmu.h"
#include "internals.h"
#include "exec/exec-all.h"
#include "qapi/error.h"
@@ -678,6 +679,16 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
set_misa(env, env->misa_mxl, ext);
}
+if (cpu->cfg.pmu_num) {
+if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) {
+cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+ riscv_pmu_timer_cb, cpu);
+if (!cpu->pmu_timer) {
+cpu->cfg.ext_sscofpmf = false;
+}
+}
+ }
+
riscv_cpu_register_gdb_regs_for_features(cs);
qemu_init_vcpu(cs);
@@ -769,6 +780,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 68522acda4d2..e2f92bb648d4 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -129,6 +129,8 @@ typedef struct PMUCTRState {
/* Snapshort value of a counter in RV32 */
target_ulong mhpmcounterh_prev;
bool started;
+/* Value beyond UINT32_MAX/UINT64_MAX before overflow interrupt trigger */
+target_ulong irq_overflow_left;
} PMUCTRState;
struct CPURISCVState {
@@ -281,6 +283,9 @@ struct CPURISCVState {
/* PMU event selector configured values. First three are unused*/
target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
+/* PMU event selector configured values for RV32*/
+target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
+
target_ulong sscratch;
target_ulong mscratch;
@@ -396,6 +401,7 @@ struct RISCVCPUConfig {
bool ext_zfhmin;
bool ext_zve32f;
bool ext_zve64f;
+bool ext_sscofpmf;
/* Vendor-specific custom extensions */
bool ext_XVentanaCondOps;
@@ -434,6 +440,12 @@ struct RISCVCPU {
/* Configuration Settings */
RISCVCPUConfig cfg;
+
+QEMUTimer *pmu_timer;
+/* A bitmask of Available programmable counters */
+uint32_t pmu_avail_ctrs;
+/* Mapping of events to counters */
+GHashTable *pmu_event_ctr_map;
};
static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)
@@ -693,6 +705,19 @@ enum {
CSR_TABLE_SIZE = 0x1000
};
+/**
+ * The event id are encoded based on the encoding specified in the
+ * SBI specification v0.3
+ */
+
+enum riscv_pmu_event_idx {
+RISCV_PMU_EVENT_HW_CPU_CYCLES = 0x01,
+RISCV_PMU_EVENT_HW_INSTRUCTIONS = 0x02,
+RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS = 0x10019,
+RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS = 0x1001B,
+RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS = 0x10021,
+};
+
/* CSR function table */
extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 48b39e6d52a7..da78e2704081 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -400,6 +400,37 @@
#define CSR_MHPMEVENT29 0x33d
#define CSR_MHPMEVENT30 0x33e
#define CSR_MHPMEVENT31 0x33f
+
+#define CSR_MHPMEVENT3H 0x723
+#define CSR_MHPMEVENT4H 0x724
+#define CSR_MHPMEVENT5H 0x725
+#define CSR_MHPMEVENT6H 0x726
+#define CSR_MHPMEVENT7H 0x727
+#define CSR_MHPMEVENT8H 0x728
+#define CSR_MHPMEVENT9H 0x729
+#define CSR_MHPMEVENT10H0x72a
+#define CSR_MHPMEVENT11H0x72b
+#define CSR_MHPMEVENT12H0x72c
+#define CSR_MHPMEVENT13H0x72d
+#define CSR_MHPMEVENT14H0x72e
+#defi
[PATCH v5 05/12] target/riscv: Implement mcountinhibit CSR
From: Atish Patra
As per the privilege specification v1.11, mcountinhibit allows to start/stop
a pmu counter selectively.
Reviewed-by: Bin Meng
Reviewed-by: Alistair Francis
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.h | 2 ++
target/riscv/cpu_bits.h | 4
target/riscv/csr.c | 25 +
target/riscv/machine.c | 1 +
4 files changed, 32 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f6b994a74ed9..ea3862ccbf5c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -259,6 +259,8 @@ struct CPURISCVState {
target_ulong scounteren;
target_ulong mcounteren;
+target_ulong mcountinhibit;
+
target_ulong sscratch;
target_ulong mscratch;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index bb47cf7e77a2..48b39e6d52a7 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -367,6 +367,10 @@
#define CSR_MHPMCOUNTER29 0xb1d
#define CSR_MHPMCOUNTER30 0xb1e
#define CSR_MHPMCOUNTER31 0xb1f
+
+/* Machine counter-inhibit register */
+#define CSR_MCOUNTINHIBIT 0x320
+
#define CSR_MHPMEVENT3 0x323
#define CSR_MHPMEVENT4 0x324
#define CSR_MHPMEVENT5 0x325
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d69bd2d88454..2283ff33a5d7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1379,6 +1379,28 @@ static RISCVException write_mtvec(CPURISCVState *env,
int csrno,
return RISCV_EXCP_NONE;
}
+static RISCVException read_mcountinhibit(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+if (env->priv_ver < PRIV_VERSION_1_11_0) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+*val = env->mcountinhibit;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+if (env->priv_ver < PRIV_VERSION_1_11_0) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+env->mcountinhibit = val;
+return RISCV_EXCP_NONE;
+}
+
static RISCVException read_mcounteren(CPURISCVState *env, int csrno,
target_ulong *val)
{
@@ -3553,6 +3575,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MHPMCOUNTER30] = { "mhpmcounter30", mctr, read_zero },
[CSR_MHPMCOUNTER31] = { "mhpmcounter31", mctr, read_zero },
+[CSR_MCOUNTINHIBIT] = { "mcountinhibit", any,read_mcountinhibit,
+ write_mcountinhibit },
+
[CSR_MHPMEVENT3] = { "mhpmevent3", any,read_zero },
[CSR_MHPMEVENT4] = { "mhpmevent4", any,read_zero },
[CSR_MHPMEVENT5] = { "mhpmevent5", any,read_zero },
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index ebc33c9e2781..a34cc3f69c4b 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -299,6 +299,7 @@ const VMStateDescription vmstate_riscv_cpu = {
VMSTATE_UINTTL(env.siselect, RISCVCPU),
VMSTATE_UINTTL(env.scounteren, RISCVCPU),
VMSTATE_UINTTL(env.mcounteren, RISCVCPU),
+VMSTATE_UINTTL(env.mcountinhibit, RISCVCPU),
VMSTATE_UINTTL(env.sscratch, RISCVCPU),
VMSTATE_UINTTL(env.mscratch, RISCVCPU),
VMSTATE_UINT64(env.mfromhost, RISCVCPU),
--
2.30.2
[PATCH v5 02/12] target/riscv: Implement PMU CSR predicate function for S-mode
From: Atish Patra
Currently, the predicate function for PMU related CSRs only works if
virtualization is enabled. It also does not check mcounteren bits before
before cycle/minstret/hpmcounterx access.
Support supervisor mode access in the predicate function as well.
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 52 ++
1 file changed, 52 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 3799ee850087..789f0c598932 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -67,12 +67,64 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
#if !defined(CONFIG_USER_ONLY)
CPUState *cs = env_cpu(env);
RISCVCPU *cpu = RISCV_CPU(cs);
+int ctr_index;
if (!cpu->cfg.ext_counters) {
/* The Counters extensions is not enabled */
return RISCV_EXCP_ILLEGAL_INST;
}
+if (env->priv == PRV_S) {
+switch (csrno) {
+case CSR_CYCLE:
+if (!get_field(env->mcounteren, COUNTEREN_CY)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+case CSR_TIME:
+if (!get_field(env->mcounteren, COUNTEREN_TM)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+case CSR_INSTRET:
+if (!get_field(env->mcounteren, COUNTEREN_IR)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
+ctr_index = csrno - CSR_CYCLE;
+if (!get_field(env->mcounteren, 1 << ctr_index)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+}
+if (riscv_cpu_is_32bit(env)) {
+switch (csrno) {
+case CSR_CYCLEH:
+if (!get_field(env->mcounteren, COUNTEREN_CY)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+case CSR_TIMEH:
+if (!get_field(env->mcounteren, COUNTEREN_TM)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+case CSR_INSTRETH:
+if (!get_field(env->mcounteren, COUNTEREN_IR)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
+ctr_index = csrno - CSR_CYCLEH;
+if (!get_field(env->mcounteren, 1 << ctr_index)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+break;
+}
+}
+}
+
if (riscv_cpu_virt_enabled(env)) {
switch (csrno) {
case CSR_CYCLE:
--
2.30.2
[PATCH v5 00/12] Improve PMU support
The latest version of the SBI specification includes a Performance Monitoring
Unit(PMU) extension[1] which allows the supervisor to start/stop/configure
various PMU events. The Sscofpmf ('Ss' for Privileged arch and Supervisor-level
extensions, and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension[2] allows the perf like tool to handle overflow interrupts and
filtering support.
This series implements full PMU infrastructure to support
PMU in virt machine. This will allow us to add any PMU events in future.
Currently, this series enables the following omu events.
1. cycle count
2. instruction count
3. DTLB load/store miss
4. ITLB prefetch miss
The first two are computed using host ticks while last three are counted during
cpu_tlb_fill. We can do both sampling and count from guest userspace.
This series has been tested on both RV64 and RV32. Both Linux[3] and Opensbi[4]
patches are required to get the perf working.
Here is an output of perf stat/report while running hackbench with OpenSBI &
Linux
kernel patches applied [3].
Perf stat:
==
[root@fedora-riscv ~]# perf stat -e cycles -e instructions -e dTLB-load-misses
-e dTLB-store-misses -e iTLB-load-misses \
> perf bench sched messaging -g 1 -l 10
# Running 'sched/messaging' benchmark:
# 20 sender and receiver processes per group
# 1 groups == 40 processes run
Total time: 0.265 [sec]
Performance counter stats for 'perf bench sched messaging -g 1 -l 10':
4,167,825,362 cycles
4,166,609,256 instructions #1.00 insn per cycle
3,092,026 dTLB-load-misses
258,280 dTLB-store-misses
2,068,966 iTLB-load-misses
0.585791767 seconds time elapsed
0.373802000 seconds user
1.042359000 seconds sys
Perf record:
[root@fedora-riscv ~]# perf record -e cycles -e instructions \
> -e dTLB-load-misses -e dTLB-store-misses -e iTLB-load-misses -c 1 \
> perf bench sched messaging -g 1 -l 10
# Running 'sched/messaging' benchmark:
# 20 sender and receiver processes per group
# 1 groups == 40 processes run
Total time: 1.397 [sec]
[ perf record: Woken up 10 times to write data ]
Check IO/CPU overload!
[ perf record: Captured and wrote 8.211 MB perf.data (214486 samples) ]
[root@fedora-riscv riscv]# perf report
Available samples
107K cycles◆
107K instructions ▒
250 dTLB-load-misses ▒
13 dTLB-store-misses ▒
172 iTLB-load-misses
..
Changes from v4->v5:
1. Rebased on top of the -next with following patches.
- isa extension
- priv 1.12 spec
2. Addressed all the comments on v4
3. Removed additional isa-ext DT node in favor of riscv,isa string update
Changes from v3->v4:
1. Removed the dummy events from pmu DT node.
2. Fixed pmu_avail_counters mask generation.
3. Added a patch to simplify the predicate function for counters.
Changes from v2->v3:
1. Addressed all the comments on PATCH1-4.
2. Split patch1 into two separate patches.
3. Added explicit comments to explain the event types in DT node.
4. Rebased on latest Qemu.
Changes from v1->v2:
1. Dropped the ACks from v1 as signficant changes happened after v1.
2. sscofpmf support.
3. A generic counter management framework.
[1] https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/riscv-sbi.adoc
[2] https://drive.google.com/file/d/171j4jFjIkKdj5LWcExphq4xG_2sihbfd/edit
[3] https://github.com/atishp04/linux/tree/riscv_pmu_v6
[4] https://github.com/atishp04/qemu/tree/riscv_pmu_v5
Atish Patra (12):
target/riscv: Fix PMU CSR predicate function
target/riscv: Implement PMU CSR predicate function for S-mode
target/riscv: pmu: Rename the counters extension to pmu
target/riscv: pmu: Make number of counters configurable
target/riscv: Implement mcountinhibit CSR
target/riscv: Add support for hpmcounters/hpmevents
target/riscv: Support mcycle/minstret write operation
target/riscv: Add sscofpmf extension support
target/riscv: Simplify counter predicate function
target/riscv: Add few cache related PMU events
hw/riscv: virt: Add PMU DT node to the device tree
target/riscv: Update the privilege field for sscofpmf CSRs
hw/riscv/virt.c | 28 ++
target/riscv/cpu.c| 15 +-
target/riscv/cpu.h| 49 ++-
target/riscv/cpu_bits.h | 59 +++
target/riscv/cpu_helper.c | 26 ++
target/riscv/csr.c| 862 --
target/riscv/machine.c| 25 ++
target/riscv/meson.build | 1 +
target/riscv/pmu.c| 431 +++
[PATCH v5 03/12] target/riscv: pmu: Rename the counters extension to pmu
From: Atish Patra
The PMU counters are supported via cpu config "Counters" which doesn't
indicate the correct purpose of those counters.
Rename the config property to pmu to indicate that these counters
are performance monitoring counters. This aligns with cpu options for
ARM architecture as well.
Reviewed-by: Bin Meng
Reviewed-by: Alistair Francis
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.c | 2 +-
target/riscv/cpu.h | 2 +-
target/riscv/csr.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 09dc07d12d6f..173b9d3c5d3e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -768,7 +768,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
-DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
+DEFINE_PROP_BOOL("pmu", RISCVCPU, cfg.ext_pmu, true),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5d914bd34550..2dc491887f24 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -365,7 +365,7 @@ struct RISCVCPUConfig {
bool ext_zbb;
bool ext_zbc;
bool ext_zbs;
-bool ext_counters;
+bool ext_pmu;
bool ext_ifencei;
bool ext_icsr;
bool ext_svinval;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 789f0c598932..2c3bba81c8af 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -69,8 +69,8 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
RISCVCPU *cpu = RISCV_CPU(cs);
int ctr_index;
-if (!cpu->cfg.ext_counters) {
-/* The Counters extensions is not enabled */
+if (!cpu->cfg.ext_pmu) {
+/* The PMU extension is not enabled */
return RISCV_EXCP_ILLEGAL_INST;
}
--
2.30.2
Re: [PATCH v2 1/2] hw/ide: split bmdma read and write functions from piix.c
On Fri, 18 Feb 2022, Liav Albani wrote:
This is a preparation before implementing another PCI IDE controller
that relies on these functions, so these can be shared between both
implementations.
Signed-off-by: Liav Albani
---
hw/ide/bmdma.c | 84 ++
hw/ide/meson.build | 2 +-
hw/ide/piix.c | 51 ++---
include/hw/ide/bmdma.h | 34 +
4 files changed, 122 insertions(+), 49 deletions(-)
create mode 100644 hw/ide/bmdma.c
create mode 100644 include/hw/ide/bmdma.h
diff --git a/hw/ide/bmdma.c b/hw/ide/bmdma.c
new file mode 100644
index 00..d12ed730dd
--- /dev/null
+++ b/hw/ide/bmdma.c
@@ -0,0 +1,84 @@
+/*
+ * QEMU IDE Emulation: Intel PCI Bus master IDE support for PIIX3/4 and ICH6/7.
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/dma.h"
+
+#include "hw/ide/bmdma.h"
+#include "hw/ide/pci.h"
+#include "trace.h"
Are you sure you need all these includes? I haven't checked but I think
there may be some unneeded ones here. On the other hand I'm not sure
putting these in a new file is worth it. There are already some bmdma_*
functions in hw/ide/pci.c which are declared in include/hw/ide/pci.h so
you could just move these there too then no new file, new header nor
changes to meson.build is needed in this patch..
+
+uint64_t intel_ide_bmdma_read(void *opaque, hwaddr addr, unsigned size)
As I said before these aren't intel specific as the same functions also
appear in other ide controllers such as via.c so maybe a better name would
be bmdma_default_read and bmdma_default_write or somehting similar so
these can be also reused by other non-intel devices too.
Regards,
BALATON Zoltan
+{
+BMDMAState *bm = opaque;
+uint32_t val;
+
+if (size != 1) {
+return ((uint64_t)1 << (size * 8)) - 1;
+}
+
+switch (addr & 3) {
+case 0:
+val = bm->cmd;
+break;
+case 2:
+val = bm->status;
+break;
+default:
+val = 0xff;
+break;
+}
+
+trace_bmdma_read(addr, val);
+return val;
+}
+
+void intel_ide_bmdma_write(void *opaque, hwaddr addr,
+uint64_t val, unsigned size)
+{
+BMDMAState *bm = opaque;
+
+if (size != 1) {
+return;
+}
+
+trace_bmdma_write(addr, val);
+
+switch (addr & 3) {
+case 0:
+bmdma_cmd_writeb(bm, val);
+break;
+case 2:
+uint8_t cur_val = bm->status;
+bm->status = (val & 0x60) | (cur_val & 1) | (cur_val & ~val & 0x06);
+break;
+}
+}
diff --git a/hw/ide/meson.build b/hw/ide/meson.build
index ddcb3b28d2..38f9ae7178 100644
--- a/hw/ide/meson.build
+++ b/hw/ide/meson.build
@@ -7,7 +7,7 @@ softmmu_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c',
'ioport.c'))
softmmu_ss.add(when: 'CONFIG_IDE_MACIO', if_true: files('macio.c'))
softmmu_ss.add(when: 'CONFIG_IDE_MMIO', if_true: files('mmio.c'))
softmmu_ss.add(when: 'CONFIG_IDE_PCI', if_true: files('pci.c'))
-softmmu_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c'))
+softmmu_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c',
'bmdma.c'))
softmmu_ss.add(when: 'CONFIG_IDE_QDEV', if_true: files('qdev.c'))
softmmu_ss.add(when: 'CONFIG_IDE_SII3112', if_true: files('sii3112.c'))
softmmu_ss.add(when: 'CONFIG_IDE_VIA', if_true: files('via.c'))
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ce89fd0aa3..8f94809eee 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -33,57 +33,12 @@
#include "sysemu/dma.h"
#include "hw/ide/pci.h"
+#include "hw/ide/bmdma.h"
#include "trace.h"
-static uint64_t bmdma_read(void *o
[PATCH v5 00/12] Improve PMU support
The latest version of the SBI specification includes a Performance Monitoring
Unit(PMU) extension[1] which allows the supervisor to start/stop/configure
various PMU events. The Sscofpmf ('Ss' for Privileged arch and Supervisor-level
extensions, and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension[2] allows the perf like tool to handle overflow interrupts and
filtering support.
This series implements full PMU infrastructure to support
PMU in virt machine. This will allow us to add any PMU events in future.
Currently, this series enables the following omu events.
1. cycle count
2. instruction count
3. DTLB load/store miss
4. ITLB prefetch miss
The first two are computed using host ticks while last three are counted during
cpu_tlb_fill. We can do both sampling and count from guest userspace.
This series has been tested on both RV64 and RV32. Both Linux[3] and Opensbi[4]
patches are required to get the perf working.
Here is an output of perf stat/report while running hackbench with OpenSBI &
Linux
kernel patches applied [3].
Perf stat:
==
[root@fedora-riscv ~]# perf stat -e cycles -e instructions -e dTLB-load-misses
-e dTLB-store-misses -e iTLB-load-misses \
> perf bench sched messaging -g 1 -l 10
# Running 'sched/messaging' benchmark:
# 20 sender and receiver processes per group
# 1 groups == 40 processes run
Total time: 0.265 [sec]
Performance counter stats for 'perf bench sched messaging -g 1 -l 10':
4,167,825,362 cycles
4,166,609,256 instructions #1.00 insn per cycle
3,092,026 dTLB-load-misses
258,280 dTLB-store-misses
2,068,966 iTLB-load-misses
0.585791767 seconds time elapsed
0.373802000 seconds user
1.042359000 seconds sys
Perf record:
[root@fedora-riscv ~]# perf record -e cycles -e instructions \
> -e dTLB-load-misses -e dTLB-store-misses -e iTLB-load-misses -c 1 \
> perf bench sched messaging -g 1 -l 10
# Running 'sched/messaging' benchmark:
# 20 sender and receiver processes per group
# 1 groups == 40 processes run
Total time: 1.397 [sec]
[ perf record: Woken up 10 times to write data ]
Check IO/CPU overload!
[ perf record: Captured and wrote 8.211 MB perf.data (214486 samples) ]
[root@fedora-riscv riscv]# perf report
Available samples
107K cycles◆
107K instructions ▒
250 dTLB-load-misses ▒
13 dTLB-store-misses ▒
172 iTLB-load-misses
..
Changes from v4->v5:
1. Rebased on top of the -next with following patches.
- isa extension
- priv 1.12 spec
2. Addressed all the comments on v4
3. Removed additional isa-ext DT node in favor of riscv,isa string update
Changes from v3->v4:
1. Removed the dummy events from pmu DT node.
2. Fixed pmu_avail_counters mask generation.
3. Added a patch to simplify the predicate function for counters.
Changes from v2->v3:
1. Addressed all the comments on PATCH1-4.
2. Split patch1 into two separate patches.
3. Added explicit comments to explain the event types in DT node.
4. Rebased on latest Qemu.
Changes from v1->v2:
1. Dropped the ACks from v1 as signficant changes happened after v1.
2. sscofpmf support.
3. A generic counter management framework.
[1] https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/riscv-sbi.adoc
[2] https://drive.google.com/file/d/171j4jFjIkKdj5LWcExphq4xG_2sihbfd/edit
[3] https://github.com/atishp04/linux/tree/riscv_pmu_v6
[4] https://github.com/atishp04/qemu/tree/riscv_pmu_v5
Atish Patra (12):
target/riscv: Fix PMU CSR predicate function
target/riscv: Implement PMU CSR predicate function for S-mode
target/riscv: pmu: Rename the counters extension to pmu
target/riscv: pmu: Make number of counters configurable
target/riscv: Implement mcountinhibit CSR
target/riscv: Add support for hpmcounters/hpmevents
target/riscv: Support mcycle/minstret write operation
target/riscv: Add sscofpmf extension support
target/riscv: Simplify counter predicate function
target/riscv: Add few cache related PMU events
hw/riscv: virt: Add PMU DT node to the device tree
target/riscv: Update the privilege field for sscofpmf CSRs
hw/riscv/virt.c | 28 ++
target/riscv/cpu.c| 15 +-
target/riscv/cpu.h| 49 ++-
target/riscv/cpu_bits.h | 59 +++
target/riscv/cpu_helper.c | 26 ++
target/riscv/csr.c| 862 --
target/riscv/machine.c| 25 ++
target/riscv/meson.build | 1 +
target/riscv/pmu.c| 431 +++
Re: [PATCH v2 3/6] hw/misc: Add a model of the Xilinx ZynqMP CRF
On Fri, Feb 18, 2022 at 01:37:51PM +, Peter Maydell wrote: > On Thu, 3 Feb 2022 at 14:01, Edgar E. Iglesias > wrote: > > > > From: "Edgar E. Iglesias" > > > > Add a model of the Xilinx ZynqMP CRF. At the moment this > > is mostly a stub model. > > > > Reviewed-by: Philippe Mathieu-Daudé > > Signed-off-by: Edgar E. Iglesias > > --- > > > > +#define TYPE_XLNX_ZYNQMP_CRF "xlnx.zynqmp_crf" > > + > > +#define XILINX_CRF(obj) \ > > + OBJECT_CHECK(XlnxZynqMPCRF, (obj), TYPE_XLNX_ZYNQMP_CRF) > > We prefer the OBJECT_DECLARE_SIMPLE_TYPE rather than directly > defining a cast macro these days. (It also provides a typedef > for you, among other things.) > > Apart from that, and dropping minimum_version_id_old, > Reviewed-by: Peter Maydell > Thanks Peter and Luc for review comments. Sorry, things have been very busy around here, I'll try to get a new version posted next week addressing all comments I've seen so far. Best regards, Edgar
Re: 'make check-acceptance' failing on s390 tests?
On 2/19/22 02:04, Peter Maydell wrote:
Hi; is anybody else seeing 'make check-acceptance' fail on some of
the s390 tests?
(009/183) tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred:
Timeout reached\nOriginal status: ERROR\n{'name':
'009-tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg',
'logdir':
'/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/tests/results/j...
(900.20 s)
(090/183)
tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora:
FAIL: b'1280 800\n' != b'1024 768\n' (26.79 s)
FWIW, yes, I'm seeing those.
r~
Re: configure: How to pass flags to the Objective-C compiler?
Hi Joshua, On 18/2/22 22:58, Joshua Seaton wrote: Hi all, How does one pass Objective-C compilation flags (e.g., a sysroot specification) when configuring/building? `configure` exposes `--extra-cflags` and `--extra-cxxflags` for C/C++ compilation, but there isn't an equivalent for Objective-C. You can use this patch (which is going to be merged soon): https://lore.kernel.org/qemu-devel/20220215080307.69550-3-f4...@amsat.org/ In my particular case, I'd like to specify a particular sysroot for a macOS build. Regards, Phil.
configure: How to pass flags to the Objective-C compiler?
Hi all, How does one pass Objective-C compilation flags (e.g., a sysroot specification) when configuring/building? `configure` exposes `--extra-cflags` and `--extra-cxxflags` for C/C++ compilation, but there isn't an equivalent for Objective-C. In my particular case, I'd like to specify a particular sysroot for a macOS build. Joshua.
Re: Call for GSoC and Outreachy project ideas for summer 2022
On 220128 1547, 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! Here are two fuzzing-related ideas: Summary: Implement rapid guest-initiated snapshot/restore functionality (for Fuzzing). Description: Many recent fuzzing projects rely on snapshot/restore functionality [1,2,3,4,5]. For example tests/fuzzers that target large targets, such as OS kernels and browsers benefit from full-VM snapshots, where solutions such as manual state-cleanup and fork-servers are insufficient. Many of the existing solutions are based on QEMU, however there is currently no upstream-solution. Furthermore, hypervisors, such as Xen have already incorporated support for snapshot-fuzzing. In this project, you will implement a virtual-device for snapshot fuzzing, following a spec agreed-upon by the community. The device will implement standard fuzzing APIs that allow fuzzing using engines, such as libFuzzer and AFL++. The simple APIs exposed by the device will allow fuzzer developers to build custom harnesses in the VM to request snapshots, memory/device/register restores, request new inputs, and report coverage. [1] https://arxiv.org/pdf/2111.03013.pdf [2] https://blog.mozilla.org/attack-and-defense/2021/01/27/effectively-fuzzing-the-ipc-layer-in-firefox/ [3] https://www.usenix.org/system/files/sec20-song.pdf [4] https://github.com/intel/kernel-fuzzer-for-xen-project [5] https://github.com/quarkslab/rewind Skill level: Intermediate with interest and experience in fuzzing. Language/Skills: C Topic/Skill Areas: Fuzzing, OS/Systems/Drivers Summary: Implement a coverage-guided fuzzer for QEMU images Description: QEMU has a qcow2 fuzzer (see tests/image-fuzzer). However, this fuzzer is not coverage-guided, and is limited to qcow2 images. Furthermore, it does not run on OSS-Fuzz. In some contexts, qemu-img is expected to handle untrusted disk images. As such, it is important to effectively fuzz this code. Your task will be to create a coverage-guided fuzzer for image formats supported by QEMU. Beyond basic image-parsing code, the fuzzer should be able to find bugs in image-conversion code. Combined with a corpus of QEMU images, the fuzzer harness will need less information about image layout. Skill level: Intermediate Language/Skills: C Topic/Skill Areas: Fuzzing, libFuzzer/AFL Thanks -Alex
[PATCH v2 2/2] hw/ide: add ich6 ide controller device emulation
This type of IDE controller has support for relocating the IO ports and
doesn't use IRQ 14 and 15 but one allocated PCI IRQ for the controller.
There's no x86 chipset in QEMU that will try to attach this device by
default. It is considered a legacy-free device in the aspect of PCI bus
resource management as the guest OS can relocate the IO ports as it sees
fit to its needs. However, this is still a legacy device that belongs to
chipsets from late 2000s.
Signed-off-by: Liav Albani
---
hw/i386/Kconfig | 2 +
hw/ide/Kconfig | 5 +
hw/ide/ich6.c| 204 +++
hw/ide/meson.build | 1 +
include/hw/ide/pci.h | 1 +
include/hw/pci/pci_ids.h | 1 +
6 files changed, 214 insertions(+)
create mode 100644 hw/ide/ich6.c
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index d22ac4a4b9..a18de2d962 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -75,6 +75,7 @@ config I440FX
select PCI_I440FX
select PIIX3
select IDE_PIIX
+select IDE_ICH6
select DIMM
select SMBIOS
select FW_CFG_DMA
@@ -101,6 +102,7 @@ config Q35
select PCI_EXPRESS_Q35
select LPC_ICH9
select AHCI_ICH9
+select IDE_ICH6
select DIMM
select SMBIOS
select FW_CFG_DMA
diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
index dd85fa3619..63304325a5 100644
--- a/hw/ide/Kconfig
+++ b/hw/ide/Kconfig
@@ -38,6 +38,11 @@ config IDE_VIA
select IDE_PCI
select IDE_QDEV
+config IDE_ICH6
+bool
+select IDE_PCI
+select IDE_QDEV
+
config MICRODRIVE
bool
select IDE_QDEV
diff --git a/hw/ide/ich6.c b/hw/ide/ich6.c
new file mode 100644
index 00..8f46d3fce2
--- /dev/null
+++ b/hw/ide/ich6.c
@@ -0,0 +1,204 @@
+/*
+ * QEMU IDE Emulation: PCI ICH6/ICH7 IDE support.
+ *
+ * Copyright (c) 2022 Liav Albani
+ *
+ * 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/pci/pci.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/dma.h"
+
+#include "hw/ide/pci.h"
+#include "hw/ide/bmdma.h"
+#include "trace.h"
+
+static const MemoryRegionOps ich6_bmdma_ops = {
+.read = intel_ide_bmdma_read,
+.write = intel_ide_bmdma_write,
+};
+
+static void bmdma_setup_bar(PCIIDEState *d)
+{
+int i;
+
+memory_region_init(&d->bmdma_bar, OBJECT(d), "ich6-bmdma-container", 16);
+for (i = 0; i < 2; i++) {
+BMDMAState *bm = &d->bmdma[i];
+
+memory_region_init_io(&bm->extra_io, OBJECT(d), &ich6_bmdma_ops, bm,
+ "ich6-bmdma", 4);
+memory_region_add_subregion(&d->bmdma_bar, i * 8, &bm->extra_io);
+memory_region_init_io(&bm->addr_ioport, OBJECT(d),
+ &bmdma_addr_ioport_ops, bm, "bmdma", 4);
+memory_region_add_subregion(&d->bmdma_bar, i * 8 + 4,
&bm->addr_ioport);
+}
+}
+
+static void ich6_pci_config_write(PCIDevice *d, uint32_t addr, uint32_t val,
+int l)
+{
+uint32_t i;
+
+pci_default_write_config(d, addr, val, l);
+
+for (i = addr; i < addr + l; i++) {
+switch (i) {
+case 0x40:
+pci_default_write_config(d, i, 0x8000, 2);
+continue;
+case 0x42:
+pci_default_write_config(d, i, 0x8000, 2);
+continue;
+}
+}
+}
+
+static void ich6_ide_reset(DeviceState *dev)
+{
+PCIIDEState *d = PCI_IDE(dev);
+PCIDevice *pd = PCI_DEVICE(d);
+uint8_t *pci_conf = pd->config;
+int i;
+
+for (i = 0; i < 2; i++) {
+ide_bus_reset(&d->bus[i]);
+}
+
+/* TODO: this is the default. do not override. */
+pci_conf[PCI_COMMAND] = 0x00;
+/* TODO: this is the default. do not override. */
+pci_conf[PCI_COMMAND + 1] = 0x00;
+/* TODO: use pci_set_word */
+pci_conf[PCI_STATUS] = PCI_STATU
[PATCH v2 1/2] hw/ide: split bmdma read and write functions from piix.c
This is a preparation before implementing another PCI IDE controller
that relies on these functions, so these can be shared between both
implementations.
Signed-off-by: Liav Albani
---
hw/ide/bmdma.c | 84 ++
hw/ide/meson.build | 2 +-
hw/ide/piix.c | 51 ++---
include/hw/ide/bmdma.h | 34 +
4 files changed, 122 insertions(+), 49 deletions(-)
create mode 100644 hw/ide/bmdma.c
create mode 100644 include/hw/ide/bmdma.h
diff --git a/hw/ide/bmdma.c b/hw/ide/bmdma.c
new file mode 100644
index 00..d12ed730dd
--- /dev/null
+++ b/hw/ide/bmdma.c
@@ -0,0 +1,84 @@
+/*
+ * QEMU IDE Emulation: Intel PCI Bus master IDE support for PIIX3/4 and ICH6/7.
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/dma.h"
+
+#include "hw/ide/bmdma.h"
+#include "hw/ide/pci.h"
+#include "trace.h"
+
+uint64_t intel_ide_bmdma_read(void *opaque, hwaddr addr, unsigned size)
+{
+BMDMAState *bm = opaque;
+uint32_t val;
+
+if (size != 1) {
+return ((uint64_t)1 << (size * 8)) - 1;
+}
+
+switch (addr & 3) {
+case 0:
+val = bm->cmd;
+break;
+case 2:
+val = bm->status;
+break;
+default:
+val = 0xff;
+break;
+}
+
+trace_bmdma_read(addr, val);
+return val;
+}
+
+void intel_ide_bmdma_write(void *opaque, hwaddr addr,
+uint64_t val, unsigned size)
+{
+BMDMAState *bm = opaque;
+
+if (size != 1) {
+return;
+}
+
+trace_bmdma_write(addr, val);
+
+switch (addr & 3) {
+case 0:
+bmdma_cmd_writeb(bm, val);
+break;
+case 2:
+uint8_t cur_val = bm->status;
+bm->status = (val & 0x60) | (cur_val & 1) | (cur_val & ~val & 0x06);
+break;
+}
+}
diff --git a/hw/ide/meson.build b/hw/ide/meson.build
index ddcb3b28d2..38f9ae7178 100644
--- a/hw/ide/meson.build
+++ b/hw/ide/meson.build
@@ -7,7 +7,7 @@ softmmu_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c',
'ioport.c'))
softmmu_ss.add(when: 'CONFIG_IDE_MACIO', if_true: files('macio.c'))
softmmu_ss.add(when: 'CONFIG_IDE_MMIO', if_true: files('mmio.c'))
softmmu_ss.add(when: 'CONFIG_IDE_PCI', if_true: files('pci.c'))
-softmmu_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c'))
+softmmu_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c',
'bmdma.c'))
softmmu_ss.add(when: 'CONFIG_IDE_QDEV', if_true: files('qdev.c'))
softmmu_ss.add(when: 'CONFIG_IDE_SII3112', if_true: files('sii3112.c'))
softmmu_ss.add(when: 'CONFIG_IDE_VIA', if_true: files('via.c'))
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ce89fd0aa3..8f94809eee 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -33,57 +33,12 @@
#include "sysemu/dma.h"
#include "hw/ide/pci.h"
+#include "hw/ide/bmdma.h"
#include "trace.h"
-static uint64_t bmdma_read(void *opaque, hwaddr addr, unsigned size)
-{
-BMDMAState *bm = opaque;
-uint32_t val;
-
-if (size != 1) {
-return ((uint64_t)1 << (size * 8)) - 1;
-}
-
-switch(addr & 3) {
-case 0:
-val = bm->cmd;
-break;
-case 2:
-val = bm->status;
-break;
-default:
-val = 0xff;
-break;
-}
-
-trace_bmdma_read(addr, val);
-return val;
-}
-
-static void bmdma_write(void *opaque, hwaddr addr,
-uint64_t val, unsigned size)
-{
-BMDMAState *bm = opaque;
-
-if (size != 1) {
-return;
-}
-
-trace_bmdma_write(addr, val);
-
-switch(addr & 3) {
-case 0:
-bmdma_cmd_writeb(bm, val);
-break;
-case 2
[PATCH v2 0/2] hw/ide: implement ich6 ide controller support
This is version 2 of this patch, this time a patch series, after following the suggestions from BALATON Zoltan. I implemented this device because I have an old machine from 2009 which has the ICH7 south bridge in it, so when I tried to run Linux on it, it booted just fine (as you might expect), but when I tried to boot with the SerenityOS kernel, it struggled to initialize the IDE controller. Therefore, upstreaming these changes might be beneficial to other OS developers and hobbyists out there, and I will use this to fix the issues within the SerenityOS kernel, without the need of preparing a bare metal setup each time I need to test the code of the kernel. Please keep in mind that while this is usable right now with the Q35 chipset, when trying to boot with an i440FX machine, SeaBIOS doesn't handle this device very well, so it tries no matter what type of IDE controller it sees to assign the IO ports to legacy values. I have a patch I wrote locally which I gladly will send to SeaBIOS, that fixes this problem by ensuring that when attaching a storage device to this controller, SeaBIOS will relocate the IO ports to other values so there's no collision with the default PIIX3/4 IDE controller. Even if SeaBIOS didn't configure this device correctly, Linux will relocate the IO ports and the user can still use the attached storage devices, given that the user managed to boot from a storage device that is not attached to the ICH6 IDE controller but to other storage controller in the system. Liav Albani (2): hw/ide: split bmdma read and write functions from piix.c hw/ide: add ich6 ide controller device emulation hw/i386/Kconfig | 2 + hw/ide/Kconfig | 5 + hw/ide/bmdma.c | 84 hw/ide/ich6.c| 204 +++ hw/ide/meson.build | 3 +- hw/ide/piix.c| 51 +- include/hw/ide/bmdma.h | 34 +++ include/hw/ide/pci.h | 1 + include/hw/pci/pci_ids.h | 1 + 9 files changed, 336 insertions(+), 49 deletions(-) create mode 100644 hw/ide/bmdma.c create mode 100644 hw/ide/ich6.c create mode 100644 include/hw/ide/bmdma.h -- 2.35.1
[PATCH] ppc/pnv: fix default PHB4 QOM hierarchy
Commit 3f4c369ea63e ("ppc/pnv: make PECs create and realize PHB4s")
changed phb4_pec code to create the default PHB4 objects in
pnv_pec_default_phb_realize(). In this process the stacks[] PEC array was
removed and each PHB4 object is tied together with its PEC via the
phb->pec pointer.
This change also broke the previous QOM hierarchy - the PHB4 objects are
being created and not being parented to their respective chips. This can
be verified by 'info pic' in a powernv9 domain with default settings.
pnv_chip_power9_pic_print_info() will fail to find the PHBs because
object_child_foreach_recursive() won't find any.
The solution is to set the parent chip and the parent bus, in the same
way done for user created PHB4 devices, for all PHB4 devices.
Fixes: 3f4c369ea63e ("ppc/pnv: make PECs create and realize PHB4s")
Signed-off-by: Daniel Henrique Barboza
---
hw/pci-host/pnv_phb4.c | 36
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index e91249ef64..846e7d0c3e 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1568,40 +1568,36 @@ static PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip,
PnvPHB4 *phb,
static void pnv_phb4_realize(DeviceState *dev, Error **errp)
{
PnvPHB4 *phb = PNV_PHB4(dev);
+PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
PCIHostState *pci = PCI_HOST_BRIDGE(dev);
XiveSource *xsrc = &phb->xsrc;
+BusState *s;
Error *local_err = NULL;
int nr_irqs;
char name[32];
-/* User created PHB */
-if (!phb->pec) {
-PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
-PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
-BusState *s;
-
-if (!chip) {
-error_setg(errp, "invalid chip id: %d", phb->chip_id);
-return;
-}
+if (!chip) {
+error_setg(errp, "invalid chip id: %d", phb->chip_id);
+return;
+}
+/* User created PHBs need to be assigned to a PEC */
+if (!phb->pec) {
phb->pec = pnv_phb4_get_pec(chip, phb, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
+}
-/*
- * Reparent user created devices to the chip to build
- * correctly the device tree.
- */
-pnv_chip_parent_fixup(chip, OBJECT(phb), phb->phb_id);
+/* Reparent the PHB to the chip to build the device tree */
+pnv_chip_parent_fixup(chip, OBJECT(phb), phb->phb_id);
-s = qdev_get_parent_bus(DEVICE(chip));
-if (!qdev_set_parent_bus(DEVICE(phb), s, &local_err)) {
-error_propagate(errp, local_err);
-return;
-}
+s = qdev_get_parent_bus(DEVICE(chip));
+if (!qdev_set_parent_bus(DEVICE(phb), s, &local_err)) {
+error_propagate(errp, local_err);
+return;
}
/* Set the "big_phb" flag */
--
2.34.1
Re: [PATCH 1/2] ui/cocoa: add option to disable left-command forwarding to guest
On 18/2/22 19:55, Peter Maydell wrote:
On Sun, 2 Jan 2022 at 17:42, Carwyn Ellis wrote:
When switching between guest and host on a Mac using command-tab the
command key is sent to the guest which can trigger functionality in the
guest OS. Specifying left-command-key=off disables forwarding this key
to the guest. Defaults to enabled.
Also updated the cocoa display documentation to reference the new
left-command-key option along with the existing show-cursor option.
Signed-off-by: Carwyn Ellis
Ccing the QAPI folks for review on the QAPI parts of this.
-- PMM
---
qapi/ui.json| 17 +
qemu-options.hx | 12
ui/cocoa.m | 8 +++-
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 2b4371da37..764480e145 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1107,6 +1107,22 @@
'data': { '*grab-on-hover' : 'bool',
'*zoom-to-fit' : 'bool' } }
+##
+# @DisplayCocoa:
+#
+# Cocoa display options.
+#
+# @left-command-key: Enable/disable forwarding of left command key to
+#guest. Allows command-tab window switching on the
+#host without sending this key to the guest when
+#"off". Defaults to "on"
+#
+# Since: 6.2.50
Not a QAPI folk, but LGTM using 7.0 here instead of 6.2.50 (this the
number of the *released* QEMU version which contains this new type).
+#
+##
+{ 'struct' : 'DisplayCocoa',
+ 'data': { '*left-command-key' : 'bool' } }
+
##
# @DisplayEGLHeadless:
#
@@ -1254,6 +1270,7 @@
'discriminator' : 'type',
'data': {
'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
+ 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
'egl-headless': { 'type': 'DisplayEGLHeadless',
'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
diff --git a/qemu-options.hx b/qemu-options.hx
index fd1f8135fb..6fa9c38c83 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1912,6 +1912,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
#if defined(CONFIG_DBUS_DISPLAY)
"-display dbus[,addr=]\n"
" [,gl=on|core|es|off][,rendernode=]\n"
+#endif
+#if defined(CONFIG_COCOA)
+"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
#endif
"-display none\n"
"select display backend type\n"
@@ -1999,6 +2002,15 @@ SRST
``charset=CP850`` for IBM CP850 encoding. The default is
``CP437``.
+``cocoa``
+Display video output in a Cocoa window. Mac only. This interface
+provides drop-down menus and other UI elements to configure and
+control the VM during runtime. Valid parameters are:
+
+``show-cursor=on|off`` : Force showing the mouse cursor
+
+``left-command-key=on|off`` : Disable forwarding left command key to
host
+
``egl-headless[,rendernode=]``
Offload all OpenGL operations to a local DRI device. For any
graphical display, this display needs to be paired with either
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 69745c483b..01045d6698 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -95,6 +95,7 @@ static DisplayChangeListener dcl = {
};
static int last_buttons;
static int cursor_hide = 1;
+static int left_command_key_enabled = 1;
static int gArgc;
static char **gArgv;
@@ -834,7 +835,8 @@ QemuCocoaView *cocoaView;
/* Don't pass command key changes to guest unless mouse is
grabbed */
case kVK_Command:
if (isMouseGrabbed &&
-!!(modifiers & NSEventModifierFlagCommand)) {
+!!(modifiers & NSEventModifierFlagCommand) &&
+left_command_key_enabled) {
[self toggleKey:Q_KEY_CODE_META_L];
}
break;
@@ -2054,6 +2056,10 @@ static void cocoa_display_init(DisplayState *ds,
DisplayOptions *opts)
cursor_hide = 0;
}
+if (opts->u.cocoa.has_left_command_key && !opts->u.cocoa.left_command_key)
{
+left_command_key_enabled = 0;
+}
+
// register vga output callbacks
register_displaychangelistener(&dcl);
Re: [PULL 00/10] Misc next patches
On Thu, 17 Feb 2022 at 12:01, Daniel P. Berrangé wrote: > > The following changes since commit ad38520bdeb2b1e0b487db317f29119e94c1c88d: > > Merge remote-tracking branch > 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2022-02-15 > 19:30:33 +) > > are available in the Git repository at: > > https://gitlab.com/berrange/qemu tags/misc-next-pull-request > > for you to fetch changes up to 2720ceda0521bc43139cfdf45e3e470559e11ce3: > > docs: expand firmware descriptor to allow flash without NVRAM (2022-02-16 > 18:53:26 +) > > > This misc series of changes: > > - Improves documentation of SSH fingerprint checking > - Fixes SHA256 fingerprints with non-blockdev usage > - Blocks the clone3, setns, unshare & execveat syscalls >with seccomp > - Blocks process spawning via clone syscall, but allows >threads, with seccomp > - Takes over seccomp maintainer role > - Expands firmware descriptor spec to allow flash >without NVRAM Hi; this series seems to cause the x64-freebsd-13-build to fail: https://gitlab.com/qemu-project/qemu/-/jobs/2112237501 1/1 qemu:block / qemu-iotests qcow2 ERROR 155.99s exit status 1 ▶ 469/707 /or1k/qmp/x-query-opcount OK ▶ 493/707 /ppc64/pnv-xscom/cfam_id/POWER8NVL OK Summary of Failures: 1/1 qemu:block / qemu-iotests qcow2 ERROR 155.99s exit status 1 Ok: 0 Expected Fail: 0 Fail: 1 Unexpected Pass: 0 Skipped: 0 Timeout: 0 Full log written to /tmp/cirrus-ci-build/build/meson-logs/iotestslog.txt This is an allowed-to-fail job, so I could in theory allow the merge, but OTOH the job was passing previously and the failure is block-related and this is a block-related pullreq... thanks -- PMM
Re: [PATCH 1/2] ui/cocoa: add option to disable left-command forwarding to guest
On Sun, 2 Jan 2022 at 17:42, Carwyn Ellis wrote:
>
> When switching between guest and host on a Mac using command-tab the
> command key is sent to the guest which can trigger functionality in the
> guest OS. Specifying left-command-key=off disables forwarding this key
> to the guest. Defaults to enabled.
>
> Also updated the cocoa display documentation to reference the new
> left-command-key option along with the existing show-cursor option.
>
> Signed-off-by: Carwyn Ellis
Ccing the QAPI folks for review on the QAPI parts of this.
-- PMM
> ---
> qapi/ui.json| 17 +
> qemu-options.hx | 12
> ui/cocoa.m | 8 +++-
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 2b4371da37..764480e145 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1107,6 +1107,22 @@
>'data': { '*grab-on-hover' : 'bool',
> '*zoom-to-fit' : 'bool' } }
>
> +##
> +# @DisplayCocoa:
> +#
> +# Cocoa display options.
> +#
> +# @left-command-key: Enable/disable forwarding of left command key to
> +#guest. Allows command-tab window switching on the
> +#host without sending this key to the guest when
> +#"off". Defaults to "on"
> +#
> +# Since: 6.2.50
> +#
> +##
> +{ 'struct' : 'DisplayCocoa',
> + 'data': { '*left-command-key' : 'bool' } }
> +
> ##
> # @DisplayEGLHeadless:
> #
> @@ -1254,6 +1270,7 @@
>'discriminator' : 'type',
>'data': {
>'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
> + 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
>'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
>'egl-headless': { 'type': 'DisplayEGLHeadless',
> 'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
> diff --git a/qemu-options.hx b/qemu-options.hx
> index fd1f8135fb..6fa9c38c83 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1912,6 +1912,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
> #if defined(CONFIG_DBUS_DISPLAY)
> "-display dbus[,addr=]\n"
> " [,gl=on|core|es|off][,rendernode=]\n"
> +#endif
> +#if defined(CONFIG_COCOA)
> +"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
> #endif
> "-display none\n"
> "select display backend type\n"
> @@ -1999,6 +2002,15 @@ SRST
> ``charset=CP850`` for IBM CP850 encoding. The default is
> ``CP437``.
>
> +``cocoa``
> +Display video output in a Cocoa window. Mac only. This interface
> +provides drop-down menus and other UI elements to configure and
> +control the VM during runtime. Valid parameters are:
> +
> +``show-cursor=on|off`` : Force showing the mouse cursor
> +
> +``left-command-key=on|off`` : Disable forwarding left command key to
> host
> +
> ``egl-headless[,rendernode=]``
> Offload all OpenGL operations to a local DRI device. For any
> graphical display, this display needs to be paired with either
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 69745c483b..01045d6698 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -95,6 +95,7 @@ static DisplayChangeListener dcl = {
> };
> static int last_buttons;
> static int cursor_hide = 1;
> +static int left_command_key_enabled = 1;
>
> static int gArgc;
> static char **gArgv;
> @@ -834,7 +835,8 @@ QemuCocoaView *cocoaView;
> /* Don't pass command key changes to guest unless mouse is
> grabbed */
> case kVK_Command:
> if (isMouseGrabbed &&
> -!!(modifiers & NSEventModifierFlagCommand)) {
> +!!(modifiers & NSEventModifierFlagCommand) &&
> +left_command_key_enabled) {
> [self toggleKey:Q_KEY_CODE_META_L];
> }
> break;
> @@ -2054,6 +2056,10 @@ static void cocoa_display_init(DisplayState *ds,
> DisplayOptions *opts)
> cursor_hide = 0;
> }
>
> +if (opts->u.cocoa.has_left_command_key &&
> !opts->u.cocoa.left_command_key) {
> +left_command_key_enabled = 0;
> +}
> +
> // register vga output callbacks
> register_displaychangelistener(&dcl);
Re: [PATCH 1/1] ui/cocoa: show/hide menu in fullscreen on mouse ungrab/grab
On 2022/01/03 20:45, Carwyn Ellis wrote:
The menu bar is only accessible when the Cocoa UI is windowed. In order
to allow the menu bar to be accessible in fullscreen mode, this change
makes the menu visible when the mouse is ungrabbed.
When the mouse is grabbed the menu is hidden again.
Signed-off-by: Carwyn Ellis
---
ui/cocoa.m | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 69745c483b..42dcf47da4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1037,7 +1037,9 @@ QemuCocoaView *cocoaView;
{
COCOA_DEBUG("QemuCocoaView: grabMouse\n");
-if (!isFullscreen) {
+if (isFullscreen) {
+[NSMenu setMenuBarVisible: FALSE];
+} else {
if (qemu_name)
[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - (Press
ctrl + alt + g to release Mouse)", qemu_name]];
else
@@ -1052,7 +1054,9 @@ QemuCocoaView *cocoaView;
{
COCOA_DEBUG("QemuCocoaView: ungrabMouse\n");
-if (!isFullscreen) {
+if (isFullscreen) {
+[NSMenu setMenuBarVisible: TRUE];
+} else {
if (qemu_name)
[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s",
qemu_name]];
else
[QemuCocoaView -toggleFullscreen:] also has the calls to [NSMenu
setMenuBarVisible:], which should be removed.
Regards,
Akihiko Odaki
Re: [PATCH 2/2] ui/cocoa: release mouse when user switches away from QEMU window
Reviewed-by: Akihiko Odaki
On 2022/01/03 2:41, Carwyn Ellis wrote:
This resolves an issue where using command-tab to switch between QEMU
and other windows on the host can leave the mouse pointer visible.
By releasing the mouse when the user switches away, the user must left
click on the QEMU window when switching back in order to hide the
pointer and return control to the guest.
This appraoch ensures that the calls to NSCursor hide and unhide are
always balanced and thus work correctly when invoked.
Signed-off-by: Carwyn Ellis
---
ui/cocoa.m | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 01045d6698..3f7af4a8fa 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1243,6 +1243,7 @@ QemuCocoaView *cocoaView;
- (void) applicationWillResignActive: (NSNotification *)aNotification
{
COCOA_DEBUG("QemuCocoaAppController: applicationWillResignActive\n");
+[cocoaView ungrabMouse];
[cocoaView raiseAllKeys];
}
@@ -2052,6 +2053,7 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
[(QemuCocoaAppController *)[[NSApplication sharedApplication]
delegate] toggleFullScreen: nil];
});
}
+
if (opts->has_show_cursor && opts->show_cursor) {
cursor_hide = 0;
}
Re: [PATCH 1/2] ui/cocoa: add option to disable left-command forwarding to guest
Reviewed-by: Akihiko Odaki
On 2022/01/03 2:41, Carwyn Ellis wrote:
When switching between guest and host on a Mac using command-tab the
command key is sent to the guest which can trigger functionality in the
guest OS. Specifying left-command-key=off disables forwarding this key
to the guest. Defaults to enabled.
Also updated the cocoa display documentation to reference the new
left-command-key option along with the existing show-cursor option.
Signed-off-by: Carwyn Ellis
---
qapi/ui.json| 17 +
qemu-options.hx | 12
ui/cocoa.m | 8 +++-
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 2b4371da37..764480e145 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1107,6 +1107,22 @@
'data': { '*grab-on-hover' : 'bool',
'*zoom-to-fit' : 'bool' } }
+##
+# @DisplayCocoa:
+#
+# Cocoa display options.
+#
+# @left-command-key: Enable/disable forwarding of left command key to
+#guest. Allows command-tab window switching on the
+#host without sending this key to the guest when
+#"off". Defaults to "on"
+#
+# Since: 6.2.50
+#
+##
+{ 'struct' : 'DisplayCocoa',
+ 'data': { '*left-command-key' : 'bool' } }
+
##
# @DisplayEGLHeadless:
#
@@ -1254,6 +1270,7 @@
'discriminator' : 'type',
'data': {
'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
+ 'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
'egl-headless': { 'type': 'DisplayEGLHeadless',
'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
diff --git a/qemu-options.hx b/qemu-options.hx
index fd1f8135fb..6fa9c38c83 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1912,6 +1912,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
#if defined(CONFIG_DBUS_DISPLAY)
"-display dbus[,addr=]\n"
" [,gl=on|core|es|off][,rendernode=]\n"
+#endif
+#if defined(CONFIG_COCOA)
+"-display cocoa[,show-cursor=on|off][,left-command-key=on|off]\n"
#endif
"-display none\n"
"select display backend type\n"
@@ -1999,6 +2002,15 @@ SRST
``charset=CP850`` for IBM CP850 encoding. The default is
``CP437``.
+``cocoa``
+Display video output in a Cocoa window. Mac only. This interface
+provides drop-down menus and other UI elements to configure and
+control the VM during runtime. Valid parameters are:
+
+``show-cursor=on|off`` : Force showing the mouse cursor
+
+``left-command-key=on|off`` : Disable forwarding left command key to
host
+
``egl-headless[,rendernode=]``
Offload all OpenGL operations to a local DRI device. For any
graphical display, this display needs to be paired with either
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 69745c483b..01045d6698 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -95,6 +95,7 @@ static DisplayChangeListener dcl = {
};
static int last_buttons;
static int cursor_hide = 1;
+static int left_command_key_enabled = 1;
static int gArgc;
static char **gArgv;
@@ -834,7 +835,8 @@ QemuCocoaView *cocoaView;
/* Don't pass command key changes to guest unless mouse is
grabbed */
case kVK_Command:
if (isMouseGrabbed &&
-!!(modifiers & NSEventModifierFlagCommand)) {
+!!(modifiers & NSEventModifierFlagCommand) &&
+left_command_key_enabled) {
[self toggleKey:Q_KEY_CODE_META_L];
}
break;
@@ -2054,6 +2056,10 @@ static void cocoa_display_init(DisplayState *ds,
DisplayOptions *opts)
cursor_hide = 0;
}
+if (opts->u.cocoa.has_left_command_key && !opts->u.cocoa.left_command_key) {
+left_command_key_enabled = 0;
+}
+
// register vga output callbacks
register_displaychangelistener(&dcl);
Re: [PATCH v6 10/15] ui/cocoa: Remove allowedFileTypes restriction in SavePanel
Reviewed-by: Akihiko Odaki
Tested-by: Akihiko Odaki
On 2022/02/15 17:03, Philippe Mathieu-Daudé via wrote:
setAllowedFileTypes is deprecated in macOS 12.
Per Akihiko Odaki [*]:
An image file, which is being chosen by the panel, can be a
raw file and have a variety of file extensions and many are not
covered by the provided list (e.g. "udf"). Other platforms like
GTK can provide an option to open a file with an extension not
listed, but Cocoa can't. It forces the user to rename the file
to give an extension in the list. Moreover, Cocoa does not tell
which extensions are in the list so the user needs to read the
source code, which is pretty bad.
Since this code is harming the usability rather than improving it,
simply remove the [NSSavePanel allowedFileTypes:] call, fixing:
[2789/6622] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
ui/cocoa.m:1411:16: error: 'setAllowedFileTypes:' is deprecated: first
deprecated in macOS 12.0 - Use -allowedContentTypes instead
[-Werror,-Wdeprecated-declarations]
[openPanel setAllowedFileTypes: supportedImageFileTypes];
^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49:
note: property 'allowedFileTypes' is declared deprecated here
@property (nullable, copy) NSArray *allowedFileTypes
API_DEPRECATED("Use -allowedContentTypes instead", macos(10.3,12.0));
^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49:
note: 'setAllowedFileTypes:' has been explicitly marked deprecated here
FAILED: libcommon.fa.p/ui_cocoa.m.o
[*]
https://lore.kernel.org/qemu-devel/4dde2e66-63cb-4390-9538-c032310db...@gmail.com/
Suggested-by: Akihiko Odaki
Reviewed-by: Roman Bolshakov
Tested-by: Roman Bolshakov
Reviewed-by: Christian Schoenebeck
Reviewed by: Cameron Esfahani
Signed-off-by: Philippe Mathieu-Daudé
---
ui/cocoa.m | 6 --
1 file changed, 6 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index ac18e14ce0..7a1ddd4075 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -100,7 +100,6 @@ static int gArgc;
static char **gArgv;
static bool stretch_video;
static NSTextField *pauseLabel;
-static NSArray * supportedImageFileTypes;
static QemuSemaphore display_init_sem;
static QemuSemaphore app_started_sem;
@@ -1168,10 +1167,6 @@ QemuCocoaView *cocoaView;
[pauseLabel setTextColor: [NSColor blackColor]];
[pauseLabel sizeToFit];
-// set the supported image file types that can be opened
-supportedImageFileTypes = [NSArray arrayWithObjects: @"img", @"iso",
@"dmg",
- @"qcow", @"qcow2", @"cloop", @"vmdk", @"cdr",
- @"toast", nil];
[self make_about_window];
}
return self;
@@ -1414,7 +1409,6 @@ QemuCocoaView *cocoaView;
openPanel = [NSOpenPanel openPanel];
[openPanel setCanChooseFiles: YES];
[openPanel setAllowsMultipleSelection: NO];
-[openPanel setAllowedFileTypes: supportedImageFileTypes];
if([openPanel runModal] == NSModalResponseOK) {
NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
if(file == nil) {
Re: [PATCH v6 08/15] audio/coreaudio: Remove a deprecation warning on macOS 12
Reviewed-by: Akihiko Odaki
Tested-by: Akihiko Odaki
On 2022/02/15 17:03, Philippe Mathieu-Daudé via wrote:
When building on macOS 12 we get:
audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is
deprecated: first deprecated in macOS 12.0 [-Werror,-Wdeprecated-declarations]
kAudioObjectPropertyElementMaster
^
kAudioObjectPropertyElementMain
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5:
note: 'kAudioObjectPropertyElementMaster' has been explicitly marked
deprecated here
kAudioObjectPropertyElementMaster
API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", macos(10.0,
12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) =
kAudioObjectPropertyElementMain
^
Replace by kAudioObjectPropertyElementMain, redefining it to
kAudioObjectPropertyElementMaster if not available.
Suggested-by: Akihiko Odaki
Suggested-by: Christian Schoenebeck
Suggested-by: Roman Bolshakov
Reviewed-by: Christian Schoenebeck
Signed-off-by: Philippe Mathieu-Daudé
---
audio/coreaudio.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index d8a21d3e50..1faef7fa7a 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -44,10 +44,15 @@ typedef struct coreaudioVoiceOut {
bool enabled;
} coreaudioVoiceOut;
+#if !defined(MAC_OS_VERSION_12_0) \
+|| (MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_VERSION_12_0)
+#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
+#endif
+
static const AudioObjectPropertyAddress voice_addr = {
kAudioHardwarePropertyDefaultOutputDevice,
kAudioObjectPropertyScopeGlobal,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
static OSStatus coreaudio_get_voice(AudioDeviceID *id)
@@ -69,7 +74,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyBufferFrameSizeRange,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectGetPropertyData(id,
@@ -86,7 +91,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id,
UInt32 *framesize)
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyBufferFrameSize,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectGetPropertyData(id,
@@ -103,7 +108,7 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id,
UInt32 *framesize)
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyBufferFrameSize,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectSetPropertyData(id,
@@ -121,7 +126,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyStreamFormat,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectSetPropertyData(id,
@@ -138,7 +143,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id,
UInt32 *result)
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyDeviceIsRunning,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectGetPropertyData(id,
Re: [PATCH 04/31] vdpa: Add vhost_svq_set_svq_kick_fd
On Tue, Feb 8, 2022 at 9:48 AM Jason Wang wrote:
>
>
> 在 2022/1/31 下午6:18, Eugenio Perez Martin 写道:
> > On Fri, Jan 28, 2022 at 7:29 AM Jason Wang wrote:
> >>
> >> 在 2022/1/22 上午4:27, Eugenio Pérez 写道:
> >>> This function allows the vhost-vdpa backend to override kick_fd.
> >>>
> >>> Signed-off-by: Eugenio Pérez
> >>> ---
> >>>hw/virtio/vhost-shadow-virtqueue.h | 1 +
> >>>hw/virtio/vhost-shadow-virtqueue.c | 45 ++
> >>>2 files changed, 46 insertions(+)
> >>>
> >>> diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> >>> b/hw/virtio/vhost-shadow-virtqueue.h
> >>> index 400effd9f2..a56ecfc09d 100644
> >>> --- a/hw/virtio/vhost-shadow-virtqueue.h
> >>> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> >>> @@ -15,6 +15,7 @@
> >>>
> >>>typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> >>>
> >>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int
> >>> svq_kick_fd);
> >>>const EventNotifier *vhost_svq_get_dev_kick_notifier(
> >>> const
> >>> VhostShadowVirtqueue *svq);
> >>>
> >>> diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> >>> b/hw/virtio/vhost-shadow-virtqueue.c
> >>> index bd87110073..21534bc94d 100644
> >>> --- a/hw/virtio/vhost-shadow-virtqueue.c
> >>> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> >>> @@ -11,6 +11,7 @@
> >>>#include "hw/virtio/vhost-shadow-virtqueue.h"
> >>>
> >>>#include "qemu/error-report.h"
> >>> +#include "qemu/main-loop.h"
> >>>
> >>>/* Shadow virtqueue to relay notifications */
> >>>typedef struct VhostShadowVirtqueue {
> >>> @@ -18,8 +19,20 @@ typedef struct VhostShadowVirtqueue {
> >>>EventNotifier hdev_kick;
> >>>/* Shadow call notifier, sent to vhost */
> >>>EventNotifier hdev_call;
> >>> +
> >>> +/*
> >>> + * Borrowed virtqueue's guest to host notifier.
> >>> + * To borrow it in this event notifier allows to register on the
> >>> event
> >>> + * loop and access the associated shadow virtqueue easily. If we use
> >>> the
> >>> + * VirtQueue, we don't have an easy way to retrieve it.
> >>> + *
> >>> + * So shadow virtqueue must not clean it, or we would lose VirtQueue
> >>> one.
> >>> + */
> >>> +EventNotifier svq_kick;
> >>>} VhostShadowVirtqueue;
> >>>
> >>> +#define INVALID_SVQ_KICK_FD -1
> >>> +
> >>>/**
> >>> * The notifier that SVQ will use to notify the device.
> >>> */
> >>> @@ -29,6 +42,35 @@ const EventNotifier *vhost_svq_get_dev_kick_notifier(
> >>>return &svq->hdev_kick;
> >>>}
> >>>
> >>> +/**
> >>> + * Set a new file descriptor for the guest to kick SVQ and notify for
> >>> avail
> >>> + *
> >>> + * @svq The svq
> >>> + * @svq_kick_fd The new svq kick fd
> >>> + */
> >>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int
> >>> svq_kick_fd)
> >>> +{
> >>> +EventNotifier tmp;
> >>> +bool check_old = INVALID_SVQ_KICK_FD !=
> >>> + event_notifier_get_fd(&svq->svq_kick);
> >>> +
> >>> +if (check_old) {
> >>> +event_notifier_set_handler(&svq->svq_kick, NULL);
> >>> +event_notifier_init_fd(&tmp,
> >>> event_notifier_get_fd(&svq->svq_kick));
> >>> +}
> >>
> >> It looks to me we don't do similar things in vhost-net. Any reason for
> >> caring about the old svq_kick?
> >>
> > Do you mean to check for old kick_fd in case we miss notifications,
> > and explicitly omit the INVALID_SVQ_KICK_FD?
>
>
> Yes.
>
>
> >
> > If you mean qemu's vhost-net, I guess it's because the device's kick
> > fd is never changed in all the vhost device lifecycle, it's only set
> > at the beginning. Previous RFC also depended on that, but you
> > suggested better vhost and SVQ in v4 feedback if I understood
> > correctly [1]. Or am I missing something?
>
>
> No, I forgot that. But in this case we should have a better dealing with
> the the conversion from valid fd to -1 by disabling the handler.
>
Sure, I will do it that way for the next version.
>
> >
> > Qemu's vhost-net does not need to use this because it is not polling
> > it. For kernel's vhost, I guess the closest is the use of pollstop and
> > pollstart at vhost_vring_ioctl.
> >
> > In my opinion, I think that SVQ code size can benefit from now
> > allowing to override kick_fd from the start of the operation. Not from
> > initialization, but start. But I can see the benefits of having the
> > change into account from this moment so it's more resilient to the
> > future.
> >
> >>> +
> >>> +/*
> >>> + * event_notifier_set_handler already checks for guest's
> >>> notifications if
> >>> + * they arrive to the new file descriptor in the switch, so there is
> >>> no
> >>> + * need to explicitely check for them.
> >>> + */
> >>> +event_notifier_init_fd(&svq->svq_kick, svq_kick_fd);
> >>> +
> >>> +if (!check_old || event_notifier_test_and_clear(&tmp)) {
> >>> +event_notifier_set(&svq->hdev_kick);
> >>
> >> Any reason
Re: [PATCH v6 11/15] ui/cocoa: Add Services menu
On Freitag, 18. Februar 2022 18:49:55 CET Akihiko Odaki wrote: > On Sat, Feb 19, 2022 at 2:33 AM Peter Maydell wrote: > > On Tue, 15 Feb 2022 at 08:26, Philippe Mathieu-Daudé via > > > > wrote: > > > From: Akihiko Odaki > > > > > > Services menu functionality of Cocoa is described at: > > > https://developer.apple.com/design/human-interface-guidelines/macos/exte > > > nsions/services/> > > I tested this, and while it does create a Services menu, none of > > the items in it seem very relevant to QEMU (on my machine, there's > > Activity Monitor, Time Profile Active Application, and some other > > software-development related things). In fact, every app I looked > > at exposed the same list of things in the Services menu. So I'm > > not sure why this is even an application-specific menu that needs > > specific code to support, rather than something system-wide that > > Apple automatically adds to the UI where it wants it. > > > > -- PMM > > Actually I wanted to use those services from Xcode to debug QEMU. I > have no idea why Apple decided to do it this way, but an application > template from Xcode does the same although it uses an interface file > instead of Objective-C code. > > Regards, > Akihiko Odaki Yes, that appearance of the "Services" menu is normal. I think the idea was to leave it completely to app developers how their app menus looks like exactly, instead of Apple injecting something there without being asked. There are much bigger oddities in macOS's menu design than that IMO. Best regards, Christian Schoenebeck
Re: [PATCH v6 00/43] CXl 2.0 emulation Support
On Fri, 11 Feb 2022 12:07:04 + Jonathan Cameron wrote: > This version is mainly a reorganization of v5 patch ordering to make for > more sensible stepwise review and merging. I took the opportunity to add > a few more tests and introduce an overview document. In case anyone is interested at: https://gitlab.com/jic23/qemu/-/commits/cxl-v7-draft-for-test is a draft of what will be v7 with basic CXL switch support and the issue that Ben pointed out in patch 17 fixed. No change to anything before patch 17 so does not affect the proposed first block of patches to apply (i.e. no need to wait for v7 if it makes sense to pick those up sooner!) There was some follow through from the fix in the later qtests as they now correctly require LSA / Label Storage Areas to be provided for CXL-mem types. I've also moved the initial type 3 test later in the series so that lsa parameter is required by the time it is introduced. The LSA requirement may be relaxed in the future when volatile memory support is added as then an LSA becomes optional. The switch support includes interleave decoding for one layer of switches and at least for basic tests it all seems to work. Note that with current kernel proposal you'll need 2 ports on a host bridge (one can have nothing attached to it) so as to ensure the kernel programs the HDM decoders. I'll see if I can find an elegant solution on the kernel side for this as single port HB with HDM decoders is allowed by the CXL spec. Next up is fleshing out sanity checks / write masks etc for various interfaces so that the emulation will reject invalid writes etc much as real hardware would be expected to do. Thanks, Jonathan > > Changes since v5: Thanks to Michael and Igor for comments. > - Reorganize series to make it more sensible to take in several chunks. > Includes splitting qtest/cxl-test up so that we have basic testing > for each stage. Suggested chunks as follows: > 1-15: Infrastructure + host bridge > At this stage the pxb-cxl host bridge it is presented to the OS > as a legacy / pcie host bridge. > This set is close to those Michael Tsirkin suggested picking up > in v5, but the cxl=on machine parameter was pulled forwards to > avoid breaking backwards compatibility. It could logically be > delayed until HB MMIO is introduced but only at cost of then > failing to instantiate pxb-cxl in cases that would previously > have 'worked'. > 16-22: Root port, plus CXL type 3 device. > Will allow Linux cxl-pci driver to bind and present some basic > features of the type 3 devices. > 23-40: x86 PC support including HB MMIO and CFMWS > At this region creation and use is possible on x86 platforms. > Includes a new bios-table-test and more complex qtests/cxl-test > cases. Includes the interleaving code and an RFC about adding > ops to memory_region_ram_init_from_file(). > 41-42: ARM virt enabling equivalent to that done for x86. > 43: Documentation > I can break this down if helpful as it includes command line > examples only possible after support is in place for all elements. > I've had gitlab CI pipelines testing at patch 15, 22, and 43 > and other than the build-oss-fuzz which is timing out and a false > positive (I think) in checkpatch they are clean. > http://gitlab.com/jic23/qemu/~/pipelines > (trivial commit message update followed pushing those out, but >the code is the same). > > - Add a documentation patch as requested by Alex in v4 review. > - Drop the code unification around PCI host bridge setup. > It will need extensive testing and was only of marginal benefit for this > series so get it off the critical path. > - Don't build CEDT unless machine parameter cxl=on is set. Tests added > to exercise the cxl=on case and generate meaningful tables (not empty). > - Move handling of pxb-cxl reset to a later patch where the required > infrastructure has been created (avoids a crash). > - Split up qtests/cxl-test to build up step by step. > Introduced an aarch64 test case covering most complex case only as > this probably sufficient to catch any arch specific problems. > - Introduce test plus bios tables CEDT.cxl and DSDT.cxl. > > Updated background info from v5: > > Looking in particular for: > * Review of the PCI interactions > * x86 and ARM machine interactions (particularly the memory maps) > * Review of the interleaving approach - is the basic idea > acceptable? > * Review of the command line interface. > * CXL related review welcome but much of that got reviewed > in earlier versions and hasn't changed substantially. > > Big TODOs: > > * Lack of unaligned accesses to interleaved memory. Are there any > side effects of this restriction? > * Volatile memory devices (easy but it's more code so left for now). > * Switch support. Linux kernel support is under review currently, > so there is now something to test against. > * Hotplug? May not need much but it's
Re: [PATCH v6 07/15] block/file-posix: Remove a deprecation warning on macOS 12
Reviewed-by: Akihiko Odaki
Tested-by: Akihiko Odaki
On 2022/02/15 17:02, Philippe Mathieu-Daudé via wrote:
When building on macOS 12 we get:
block/file-posix.c:3335:18: warning: 'IOMasterPort' is deprecated: first
deprecated in macOS 12.0 [-Wdeprecated-declarations]
kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort );
^~~~
IOMainPort
Replace by IOMainPort, redefining it to IOMasterPort if not available.
Suggested-by: Akihiko Odaki
Reviewed-by: Christian Schoenebeck
Reviewed by: Cameron Esfahani
Signed-off-by: Philippe Mathieu-Daudé
---
block/file-posix.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 1f1756e192..13393ad296 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3319,17 +3319,23 @@ BlockDriver bdrv_file = {
#if defined(__APPLE__) && defined(__MACH__)
static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
CFIndex maxPathSize, int flags);
+
+#if !defined(MAC_OS_VERSION_12_0) \
+|| (MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_VERSION_12_0)
+#define IOMainPort IOMasterPort
+#endif
+
static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator)
{
kern_return_t kernResult = KERN_FAILURE;
-mach_port_t masterPort;
+mach_port_t mainPort;
CFMutableDictionaryRef classesToMatch;
const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass};
char *mediaType = NULL;
-kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort );
+kernResult = IOMainPort(MACH_PORT_NULL, &mainPort);
if ( KERN_SUCCESS != kernResult ) {
-printf( "IOMasterPort returned %d\n", kernResult );
+printf("IOMainPort returned %d\n", kernResult);
}
int index;
@@ -3342,7 +3348,7 @@ static char *FindEjectableOpticalMedia(io_iterator_t
*mediaIterator)
}
CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey),
kCFBooleanTrue);
-kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch,
+kernResult = IOServiceGetMatchingServices(mainPort, classesToMatch,
mediaIterator);
if (kernResult != KERN_SUCCESS) {
error_report("Note: IOServiceGetMatchingServices returned %d",
Re: [PATCH v6 03/15] tests/fp/berkeley-testfloat-3: Ignore ignored #pragma directives
Reviewed-by: Akihiko Odaki Tested-by: Akihiko Odaki On 2022/02/15 17:02, Philippe Mathieu-Daudé via wrote: Since we already use -Wno-unknown-pragmas, we can also use -Wno-ignored-pragmas. This silences hundred of warnings using clang 13 on macOS Monterey: [409/771] Compiling C object tests/fp/libtestfloat.a.p/berkeley-testfloat-3_source_test_az_f128_rx.c.o ../tests/fp/berkeley-testfloat-3/source/test_az_f128_rx.c:49:14: warning: '#pragma FENV_ACCESS' is not supported on this target - ignored [-Wignored-pragmas] #pragma STDC FENV_ACCESS ON ^ 1 warning generated. Having: $ cc -v Apple clang version 13.0.0 (clang-1300.0.29.30) Signed-off-by: Philippe Mathieu-Daudé --- tests/fp/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/fp/meson.build b/tests/fp/meson.build index 59776a00a7..5192264a08 100644 --- a/tests/fp/meson.build +++ b/tests/fp/meson.build @@ -30,6 +30,7 @@ tfcflags = [ '-Wno-implicit-fallthrough', '-Wno-strict-prototypes', '-Wno-unknown-pragmas', + '-Wno-ignored-pragmas', '-Wno-uninitialized', '-Wno-missing-prototypes', '-Wno-return-type',
Re: [PATCH v6 02/15] configure: Allow passing extra Objective C compiler flags
Reviewed-by: Akihiko Odaki
Tested-by: Akihiko Odaki
On 2022/02/15 17:02, Philippe Mathieu-Daudé via wrote:
We can pass C/CPP/LD flags via CFLAGS/CXXFLAGS/LDFLAGS environment
variables, or via configure --extra-cflags / --extra-cxxflags /
--extra-ldflags options. Provide similar behavior for Objective C:
use existing flags from $OBJCFLAGS, or passed via --extra-objcflags.
Signed-off-by: Philippe Mathieu-Daudé
---
configure | 8
meson.build | 5 +
2 files changed, 13 insertions(+)
diff --git a/configure b/configure
index 3a29eff5cc..06c03cebd3 100755
--- a/configure
+++ b/configure
@@ -287,6 +287,7 @@ done
EXTRA_CFLAGS=""
EXTRA_CXXFLAGS=""
+EXTRA_OBJCFLAGS=""
EXTRA_LDFLAGS=""
xen_ctrl_version="$default_feature"
@@ -391,9 +392,12 @@ for opt do
--extra-cflags=*)
EXTRA_CFLAGS="$EXTRA_CFLAGS $optarg"
EXTRA_CXXFLAGS="$EXTRA_CXXFLAGS $optarg"
+EXTRA_OBJCFLAGS="$EXTRA_OBJCFLAGS $optarg"
;;
--extra-cxxflags=*) EXTRA_CXXFLAGS="$EXTRA_CXXFLAGS $optarg"
;;
+ --extra-objcflags=*) EXTRA_OBJCFLAGS="$EXTRA_OBJCFLAGS $optarg"
+ ;;
--extra-ldflags=*) EXTRA_LDFLAGS="$EXTRA_LDFLAGS $optarg"
;;
--enable-debug-info) debug_info="yes"
@@ -774,6 +778,8 @@ for opt do
;;
--extra-cxxflags=*)
;;
+ --extra-objcflags=*)
+ ;;
--extra-ldflags=*)
;;
--enable-debug-info)
@@ -1312,6 +1318,7 @@ Advanced options (experts only):
--objcc=OBJCCuse Objective-C compiler OBJCC [$objcc]
--extra-cflags=CFLAGSappend extra C compiler flags CFLAGS
--extra-cxxflags=CXXFLAGS append extra C++ compiler flags CXXFLAGS
+ --extra-objcflags=OBJCFLAGS append extra Objective C compiler flags OBJCFLAGS
--extra-ldflags=LDFLAGS append extra linker flags LDFLAGS
--cross-cc-ARCH=CC use compiler when building ARCH guest test cases
--cross-cc-cflags-ARCH= use compiler flags when building ARCH guest tests
@@ -3724,6 +3731,7 @@ if test "$skip_meson" = no; then
echo "[built-in options]" >> $cross
echo "c_args = [$(meson_quote $CFLAGS $EXTRA_CFLAGS)]" >> $cross
echo "cpp_args = [$(meson_quote $CXXFLAGS $EXTRA_CXXFLAGS)]" >> $cross
+ test -n "$objcc" && echo "objc_args = [$(meson_quote $OBJCFLAGS
$EXTRA_OBJCFLAGS)]" >> $cross
echo "c_link_args = [$(meson_quote $CFLAGS $LDFLAGS $EXTRA_CFLAGS
$EXTRA_LDFLAGS)]" >> $cross
echo "cpp_link_args = [$(meson_quote $CXXFLAGS $LDFLAGS $EXTRA_CXXFLAGS
$EXTRA_LDFLAGS)]" >> $cross
echo "[binaries]" >> $cross
diff --git a/meson.build b/meson.build
index ae5f7eec6e..df25e7a5e7 100644
--- a/meson.build
+++ b/meson.build
@@ -3292,6 +3292,11 @@ if link_language == 'cpp'
+ ['-O' +
get_option('optimization')]
+ (get_option('debug') ?
['-g'] : []))}
endif
+if targetos == 'darwin'
+ summary_info += {'OBJCFLAGS': ' '.join(get_option('objc_args')
+ + ['-O' +
get_option('optimization')]
+ + (get_option('debug') ? ['-g']
: []))}
+endif
link_args = get_option(link_language + '_link_args')
if link_args.length() > 0
summary_info += {'LDFLAGS': ' '.join(link_args)}
Re: [PATCH v2 7/7] aspeed/sdmc: Add trace events
On 2/18/22 18:56, Philippe Mathieu-Daudé wrote: On 18/2/22 09:18, Cédric Le Goater wrote: This is useful to analyze changes in the U-Boot RAM driver when SDRAM training is performed. Signed-off-by: Cédric Le Goater --- hw/misc/aspeed_sdmc.c | 2 ++ hw/misc/trace-events | 4 2 files changed, 6 insertions(+) diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index 08f856cbda7e..d2a3931033b3 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -130,6 +130,7 @@ static uint64_t aspeed_sdmc_read(void *opaque, hwaddr addr, unsigned size) return 0; } +# aspeed_sdmc.c +aspeed_sdmc_write(uint32_t reg, uint32_t data) "reg @0x%" PRIx32 " data: 0x%" PRIx32 +aspeed_sdmc_read(uint32_t reg, uint32_t data) "reg @0x%" PRIx32 " data: 0x%" PRIx32 I'm surprised by the uint32_t conversion, but why not. Reviewed-by: Philippe Mathieu-Daudé Indeed. I might change that. Thanks ! C.
Re: [PATCH v2 7/7] aspeed/sdmc: Add trace events
On 18/2/22 09:18, Cédric Le Goater wrote: This is useful to analyze changes in the U-Boot RAM driver when SDRAM training is performed. Signed-off-by: Cédric Le Goater --- hw/misc/aspeed_sdmc.c | 2 ++ hw/misc/trace-events | 4 2 files changed, 6 insertions(+) diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index 08f856cbda7e..d2a3931033b3 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -130,6 +130,7 @@ static uint64_t aspeed_sdmc_read(void *opaque, hwaddr addr, unsigned size) return 0; } +# aspeed_sdmc.c +aspeed_sdmc_write(uint32_t reg, uint32_t data) "reg @0x%" PRIx32 " data: 0x%" PRIx32 +aspeed_sdmc_read(uint32_t reg, uint32_t data) "reg @0x%" PRIx32 " data: 0x%" PRIx32 I'm surprised by the uint32_t conversion, but why not. Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 5/7] aspeed: Introduce a create_pca9552() helper
On 18/2/22 09:18, Cédric Le Goater wrote:
This unifies the way we create the pca9552 devices on the different boards.
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Cédric Le Goater
---
hw/arm/aspeed.c | 49 +++--
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index f71a5d87473f..11558b327bc9 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -533,6 +533,12 @@ static void romulus_bmc_i2c_init(AspeedMachineState *bmc)
i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 11), "ds1338",
0x32);
}
+static void create_pca9552(AspeedSoCState *soc, int bus_id, int addr)
+{
+i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, bus_id),
+TYPE_PCA9552, addr);
+}
Hmm in case you respin, this is an opportunity to return the I2CSlave*
pointer here.
Re: [PATCH v7 00/11] 9p: Add support for darwin
Excellent, thanks so much for the update. I'll wait till the other 9p pull
gets integrated, then rebase and test!
On Fri, Feb 18, 2022 at 12:45 PM Christian Schoenebeck <
qemu_...@crudebyte.com> wrote:
> On Freitag, 18. Februar 2022 18:04:24 CET Will Cohen wrote:
> > On Tue, Feb 15, 2022 at 2:04 PM Will Cohen wrote:
> > > This is a followup to
> > > https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg02313.html,
> > > adding 9p server support for Darwin.
> > >
> > > Since v6, the following changes have been made to the following
> patches:
> > >
> > > Patch 9/11: 9p: darwin: Implement compatibility for mknodat
> > > - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
> > >
> > > presence in osdep.h and os-posix.c
> > >
> > > 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.34.1
> >
> > As a brief additional note, this patch set has gotten a moderate amount
> of
> > performance testing downstream by various end users of podman, with
> > favorable results:
> > https://github.com/containers/podman/issues/8016#issuecomment-1044843948
>
> Thanks for sharing!
>
> Additionally you might be interested to know what's pending in the
> pipeline on
> Linux kernel side which brings a huge performance improvement with 9p:
>
> https://lore.kernel.org/netdev/cover.1640870037.git.linux_...@crudebyte.com/
>
> As for the status on this macOS series here: I will get back on it next
> week.
> From my PoV it looks fine now. So for the other gentlemen here: please
> raise
> your hand if you still find something.
>
> Will, one more thing though: I just sent a PR with one particular patch
> that I
> would ask you to test in conjunction with this series:
>
>
> https://github.com/cschoenebeck/qemu/commit/e64e27d5cb103b7764f1a05b6eda7e7fedd517c5
>
> You might simply wait for the PR being merged (maybe monday?) and then
> rebase
> this series to master and test if this patch is not breaking anything for
> macOS. Theoretically the new qemu_dirent_dup() function should fallback on
> macOS to its portable branch, but it should be tested before merging this
> series, just to be sure.
>
> Best regards,
> Christian Schoenebeck
>
>
>
Re: [PATCH v2 5/7] aspeed: Introduce a create_pca9552() helper
On 18/2/22 09:18, Cédric Le Goater wrote: This unifies the way we create the pca9552 devices on the different boards. Suggested-by: Philippe Mathieu-Daudé Signed-off-by: Cédric Le Goater --- hw/arm/aspeed.c | 49 +++-- 1 file changed, 27 insertions(+), 22 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v6 11/15] ui/cocoa: Add Services menu
On Sat, Feb 19, 2022 at 2:33 AM Peter Maydell wrote: > > On Tue, 15 Feb 2022 at 08:26, Philippe Mathieu-Daudé via > wrote: > > > > From: Akihiko Odaki > > > > Services menu functionality of Cocoa is described at: > > https://developer.apple.com/design/human-interface-guidelines/macos/extensions/services/ > > > > I tested this, and while it does create a Services menu, none of > the items in it seem very relevant to QEMU (on my machine, there's > Activity Monitor, Time Profile Active Application, and some other > software-development related things). In fact, every app I looked > at exposed the same list of things in the Services menu. So I'm > not sure why this is even an application-specific menu that needs > specific code to support, rather than something system-wide that > Apple automatically adds to the UI where it wants it. > > -- PMM Actually I wanted to use those services from Xcode to debug QEMU. I have no idea why Apple decided to do it this way, but an application template from Xcode does the same although it uses an interface file instead of Objective-C code. Regards, Akihiko Odaki
Re: [PATCH 1/5] hw/riscv/riscv_hart: free the harts array when the object is finalized
On Fri, 18 Feb 2022 at 17:39, Damien Hedde wrote: > You're right. I was confused when re-writing the message. > This leaks happen on > init -> realize-failure -> finalize > Because the array is allocated, then every cpu is initialized (and an > error failure may happen for any of them). "Failure during realize" is one of those cases I don't think we handle very well. I'd like to see a view by one of our QOM experts on what the best way to handle this is -- should one do the cleanup in realize itself, or in instance_finalize? Do the sub-objects that are being initialized and realized need to be manually cleaned up in the realize-is-failing case, or is that part automatic? Which is to say that maybe this patch is the best way to do this, but it would be nice to be sure about that... thanks -- PMM
Re: [PATCH v7 00/11] 9p: Add support for darwin
On Freitag, 18. Februar 2022 18:04:24 CET Will Cohen wrote:
> On Tue, Feb 15, 2022 at 2:04 PM Will Cohen wrote:
> > This is a followup to
> > https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg02313.html,
> > adding 9p server support for Darwin.
> >
> > Since v6, the following changes have been made to the following patches:
> >
> > Patch 9/11: 9p: darwin: Implement compatibility for mknodat
> > - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
> >
> > presence in osdep.h and os-posix.c
> >
> > 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.34.1
>
> As a brief additional note, this patch set has gotten a moderate amount of
> performance testing downstream by various end users of podman, with
> favorable results:
> https://github.com/containers/podman/issues/8016#issuecomment-1044843948
Thanks for sharing!
Additionally you might be interested to know what's pending in the pipeline on
Linux kernel side which brings a huge performance improvement with 9p:
https://lore.kernel.org/netdev/cover.1640870037.git.linux_...@crudebyte.com/
As for the status on this macOS series here: I will get back on it next week.
>From my PoV it looks fine now. So for the other gentlemen here: please raise
your hand if you still find something.
Will, one more thing though: I just sent a PR with one particular patch that I
would ask you to test in conjunction with this series:
https://github.com/cschoenebeck/qemu/commit/e64e27d5cb103b7764f1a05b6eda7e7fedd517c5
You might simply wait for the PR being merged (maybe monday?) and then rebase
this series to master and test if this patch is not breaking anything for
macOS. Theoretically the new qemu_dirent_dup() function should fallback on
macOS to its portable branch, but it should be tested before merging this
series, just to be sure.
Best regards,
Christian Schoenebeck
Re: [PATCH 1/5] hw/riscv/riscv_hart: free the harts array when the object is finalized
On 2/18/22 18:23, Peter Maydell wrote: On Fri, 18 Feb 2022 at 16:53, Damien Hedde wrote: The array is dynamically allocated by realize() depending on the number of harts. This clean-up removes memory leaks which would happen in the 'init->finalize' life-cycle use-case (happening when user creation is allowed). If the allocation happens in realize, then it won't hapen in an init->finalize cycle, only in init->realize->unrealize->finalize... -- PMM You're right. I was confused when re-writing the message. This leaks happen on init -> realize-failure -> finalize Because the array is allocated, then every cpu is initialized (and an error failure may happen for any of them). Thanks, -- Damien
Re: [PATCH v3] arm: Remove swift-bmc machine
On 17/2/22 11:31, Joel Stanley wrote: It was scheduled for removal in 7.0. Signed-off-by: Joel Stanley -- v2: also remove from docs/about/deprecated.rst v3: remove strap define, add note to removed-features.rst --- docs/about/deprecated.rst | 7 docs/about/removed-features.rst | 5 +++ docs/system/arm/aspeed.rst | 1 - hw/arm/aspeed.c | 64 - 4 files changed, 5 insertions(+), 72 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v8 5/5] multifd: Implement zero copy write in multifd migration (multifd-zero-copy)
Leonardo Bras Soares Passos wrote:
> Hello Peter, thanks for reviewing!
>
> On Mon, Feb 7, 2022 at 11:22 PM Peter Xu wrote:
>>
>> On Tue, Feb 01, 2022 at 03:29:03AM -0300, Leonardo Bras wrote:
>> > -void multifd_send_sync_main(QEMUFile *f)
>> > +int multifd_send_sync_main(QEMUFile *f)
>> > {
>> > int i;
>> > +bool flush_zero_copy;
>> >
>> > if (!migrate_use_multifd()) {
>> > -return;
>> > +return 0;
>> > }
>> > if (multifd_send_state->pages->num) {
>> > if (multifd_send_pages(f) < 0) {
>> > error_report("%s: multifd_send_pages fail", __func__);
>> > -return;
>> > +return 0;
>>
>> I've not checked how it used to do if multifd_send_pages() failed, but..
>> should
>> it returns -1 rather than 0 when there will be a return code?
>
> Yeah, that makes sense.
> The point here is that I was trying not to modify much of the current
> behavior.
if (qatomic_read(&multifd_send_state->exiting)) {
return -1;
}
if (p->quit) {
error_report("%s: channel %d has already quit!", __func__, i);
qemu_mutex_unlock(&p->mutex);
return -1;
}
This are the only two cases where the current code can return one
error. In the 1st case we are exiting, we are already in the middle of
finishing, so we don't really care.
In the second one, we have already quit, and error as already quite big.
But I agree with both comments:
- we need to improve the error paths
- leonardo changes don't affect what is already there.
> I mean, multifd_send_sync_main() would previously return void, so any
> other errors would not matter to the caller of this function, which
> will continue to run as if nothing happened.
>
> Now, if it fails with flush_zero_copy, the operation needs to be aborted.
>
> Maybe, I should make it different:
> - In any error, return -1.
> - Create/use a specific error code in the case of a failing
> flush_zero_copy, so I can test the return value for it on the caller
> function and return early.
We need to add the check. It don't matter if the problem is zero_copy
or the existing one, we are under a minor catastrophe and migration has
to be aborted.
> Or alternatively, the other errors could also return early, but since
> this will change how the code currently works, I would probably need
> another patch for that change. (so it can be easily reverted if
> needed)
>
> What do you think is better?
>
>
>> > }
>> > }
>> > +
>> > +/*
>> > + * When using zero-copy, it's necessary to flush after each iteration
>> > to
>> > + * make sure pages from earlier iterations don't end up replacing
>> > newer
>> > + * pages.
>> > + */
>> > +flush_zero_copy = migrate_use_zero_copy_send();
>> > +
>> > for (i = 0; i < migrate_multifd_channels(); i++) {
>> > MultiFDSendParams *p = &multifd_send_state->params[i];
>> >
>> > @@ -591,7 +600,7 @@ void multifd_send_sync_main(QEMUFile *f)
>> > if (p->quit) {
>> > error_report("%s: channel %d has already quit", __func__, i);
>> > qemu_mutex_unlock(&p->mutex);
>> > -return;
>> > +return 0;
>>
>> Same question here.
>
> Please see above,
>
>>
>> > }
>>
>> The rest looks good. Thanks,
Later, Juan.
Re: [PATCH 2/2] qapi: Fix stale reference to scripts/qapi.py in a comment
On 18/2/22 15:55, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- qapi/qapi-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v6 11/15] ui/cocoa: Add Services menu
On Tue, 15 Feb 2022 at 08:26, Philippe Mathieu-Daudé via wrote: > > From: Akihiko Odaki > > Services menu functionality of Cocoa is described at: > https://developer.apple.com/design/human-interface-guidelines/macos/extensions/services/ > I tested this, and while it does create a Services menu, none of the items in it seem very relevant to QEMU (on my machine, there's Activity Monitor, Time Profile Active Application, and some other software-development related things). In fact, every app I looked at exposed the same list of things in the Services menu. So I'm not sure why this is even an application-specific menu that needs specific code to support, rather than something system-wide that Apple automatically adds to the UI where it wants it. -- PMM
Re: [PATCH 1/5] hw/riscv/riscv_hart: free the harts array when the object is finalized
On Fri, 18 Feb 2022 at 16:53, Damien Hedde wrote: > > The array is dynamically allocated by realize() depending on the > number of harts. > > This clean-up removes memory leaks which would happen in the > 'init->finalize' life-cycle use-case (happening when user creation > is allowed). If the allocation happens in realize, then it won't hapen in an init->finalize cycle, only in init->realize->unrealize->finalize... -- PMM
Re: [PATCH RFCv2 2/4] i386/pc: relocate 4g start to 1T where applicable
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).
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,
x86ms->below_4g_mem_size,
x86ms->above_4g_mem_size,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1780f79bc127..b7cf44d4755e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -203,12 +203,13 @@ static void pc_q35_init(MachineState *machine)
pcms->smbios_entry_point_type);
}
-/* allocate ram and load rom/bios */
-pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
Re: [PATCH v8 5/5] multifd: Implement zero copy write in multifd migration (multifd-zero-copy)
Leonardo Bras wrote:
> Implement zero copy send on nocomp_send_write(), by making use of QIOChannel
> writev + flags & flush interface.
>
> Change multifd_send_sync_main() so flush_zero_copy() can be called
> after each iteration in order to make sure all dirty pages are sent before
> a new iteration is started. It will also flush at the beginning and at the
> end of migration.
>
> Also make it return -1 if flush_zero_copy() fails, in order to cancel
> the migration process, and avoid resuming the guest in the target host
> without receiving all current RAM.
>
> This will work fine on RAM migration because the RAM pages are not usually
> freed,
> and there is no problem on changing the pages content between
> writev_zero_copy() and
> the actual sending of the buffer, because this change will dirty the page and
> cause it to be re-sent on a next iteration anyway.
>
> A lot of locked memory may be needed in order to use multid migration
^^
multifd.
I can fix it on the commit.
> @@ -1479,7 +1479,16 @@ static bool migrate_params_check(MigrationParameters
> *params, Error **errp)
> error_prepend(errp, "Invalid mapping given for block-bitmap-mapping:
> ");
> return false;
> }
> -
> +#ifdef CONFIG_LINUX
> +if (params->zero_copy_send &&
> +(!migrate_use_multifd() ||
> + params->multifd_compression != MULTIFD_COMPRESSION_NONE ||
> + (params->tls_creds && *params->tls_creds))) {
> +error_setg(errp,
> + "Zero copy only available for non-compressed non-TLS
> multifd migration");
> +return false;
> +}
> +#endif
> return true;
> }
Test is long, but it is exactly what we need. Good.
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 43998ad117..2d68b9cf4f 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -568,19 +568,28 @@ void multifd_save_cleanup(void)
> multifd_send_state = NULL;
> }
>
> -void multifd_send_sync_main(QEMUFile *f)
> +int multifd_send_sync_main(QEMUFile *f)
> {
> int i;
> +bool flush_zero_copy;
>
> if (!migrate_use_multifd()) {
> -return;
> +return 0;
> }
> if (multifd_send_state->pages->num) {
> if (multifd_send_pages(f) < 0) {
> error_report("%s: multifd_send_pages fail", __func__);
> -return;
> +return 0;
> }
> }
> +
> +/*
> + * When using zero-copy, it's necessary to flush after each iteration to
> + * make sure pages from earlier iterations don't end up replacing newer
> + * pages.
> + */
> +flush_zero_copy = migrate_use_zero_copy_send();
> +
> for (i = 0; i < migrate_multifd_channels(); i++) {
> MultiFDSendParams *p = &multifd_send_state->params[i];
>
> @@ -591,7 +600,7 @@ void multifd_send_sync_main(QEMUFile *f)
> if (p->quit) {
> error_report("%s: channel %d has already quit", __func__, i);
> qemu_mutex_unlock(&p->mutex);
> -return;
> +return 0;
> }
>
> p->packet_num = multifd_send_state->packet_num++;
> @@ -602,6 +611,17 @@ void multifd_send_sync_main(QEMUFile *f)
> ram_counters.transferred += p->packet_len;
> qemu_mutex_unlock(&p->mutex);
> qemu_sem_post(&p->sem);
> +
> +if (flush_zero_copy) {
> +int ret;
> +Error *err = NULL;
> +
> +ret = qio_channel_flush(p->c, &err);
> +if (ret < 0) {
> +error_report_err(err);
> +return -1;
> +}
> +}
> }
> for (i = 0; i < migrate_multifd_channels(); i++) {
> MultiFDSendParams *p = &multifd_send_state->params[i];
> @@ -610,6 +630,8 @@ void multifd_send_sync_main(QEMUFile *f)
> qemu_sem_wait(&p->sem_sync);
> }
> trace_multifd_send_sync_main(multifd_send_state->packet_num);
> +
> +return 0;
> }
We are leaving pages is flight for potentially a lot of time. I *think*
that we can sync shorter than that.
> static void *multifd_send_thread(void *opaque)
> @@ -668,8 +690,8 @@ static void *multifd_send_thread(void *opaque)
> p->iov[0].iov_len = p->packet_len;
> p->iov[0].iov_base = p->packet;
>
> -ret = qio_channel_writev_all(p->c, p->iov, p->iovs_num,
> - &local_err);
> +ret = qio_channel_writev_full_all(p->c, p->iov, p->iovs_num,
> NULL,
> + 0, p->write_flags, &local_err);
> if (ret != 0) {
> break;
> }
I still think that it would be better to have a sync here in each
thread. I don't know the size, but once every couple megabytes of RAM
or so.
I did a change on:
commit d48c3a044537689866fe44e65d24c7d39a68868a
Author: Juan Quintela
Date: Fri Nov 19 15:35:58 2021 +0100
multifd
Re: [PATCH 2/2] Allow VIRTIO_F_IN_ORDER to be negotiated for vdpa devices
On Fri, Feb 18, 2022 at 11:24:23AM +0100, Eugenio Perez Martin wrote: On Thu, Feb 17, 2022 at 3:29 PM Stefano Garzarella wrote: On Thu, Feb 17, 2022 at 02:32:48AM -0500, 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. FYI Jason proposed to support VIRTIO_F_IN_ORDER in QEMU/Linux as an idea for the next GSoC/Outreachy. I offered to mentor and prepared a project description [1], if anyone else is interested, it would be great to have a co-mentor :-) I'd be happy to co-mentor for sure, should I edit the wiki to reflect it? Great :-) Yes, please edit the wiki page here: https://wiki.qemu.org/Internships/ProjectIdeas/VIRTIO_F_IN_ORDER Thanks, Stefano
Re: [PATCH v7 00/11] 9p: Add support for darwin
On Tue, Feb 15, 2022 at 2:04 PM Will Cohen wrote:
> This is a followup to
> https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg02313.html,
> adding 9p server support for Darwin.
>
> Since v6, the following changes have been made to the following patches:
>
> Patch 9/11: 9p: darwin: Implement compatibility for mknodat
> - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
> presence in osdep.h and os-posix.c
>
> 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.34.1
>
>
As a brief additional note, this patch set has gotten a moderate amount of
performance testing downstream by various end users of podman, with
favorable results:
https://github.com/containers/podman/issues/8016#issuecomment-1044843948
[PATCH 4/5] hw/intc/riscv_aclint: swi: report errors and free allocated memory
Add the instance_finalize() method to free the allocated arrays
and make realize() method report errors instead of exiting.
This commit also move the 'num-harts' property check from
riscv_aclint_swi_create() to realize().
Code in realize() is re-ordered to first check for errors and leave
the object in a clean state. To achieve this, we do the following:
+ parse_hart_config and char_to_mode are refactored to return errors
instead of exiting.
+ in case of interrupt claim failure, we release the succesfully
claimed ones.
These clean-ups allow the following life-cycle use cases (happening
when user creation is allowed) to execute as expected:
+ init -> finalize
+ init -> realize-failure -> finalize
Signed-off-by: Damien Hedde
---
hw/intc/riscv_aclint.c | 54 +++---
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index f1a5d3d284..4f66eeed0b 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -389,22 +389,48 @@ static void riscv_aclint_swi_realize(DeviceState *dev,
Error **errp)
RISCVAclintSwiState *swi = RISCV_ACLINT_SWI(dev);
int i;
+if (swi->num_harts > RISCV_ACLINT_MAX_HARTS) {
+error_setg(errp, "invalid 'num-harts': max is %u",
+ RISCV_ACLINT_MAX_HARTS);
+return;
+}
+
+/*
+ * Claim software interrupt bits:
+ * + sswi==false -> MSIP
+ * + sswi==true -> SSIP
+ * We don't claim mip.SSIP because it is writeable by software
+ */
+if (!swi->sswi) {
+for (i = 0; i < swi->num_harts; i++) {
+RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(swi->hartid_base + i));
+if (riscv_cpu_claim_interrupts(cpu, MIP_MSIP) < 0) {
+error_setg(errp, "MSIP (hartid %u) already claimed",
+ (unsigned) (swi->hartid_base + i));
+/* release interrupts we already claimed */
+while (--i >= 0) {
+cpu = RISCV_CPU(qemu_get_cpu(swi->hartid_base + i));
+riscv_cpu_release_claimed_interrupts(cpu, MIP_MSIP);
+}
+return;
+}
+}
+}
+
memory_region_init_io(&swi->mmio, OBJECT(dev), &riscv_aclint_swi_ops, swi,
TYPE_RISCV_ACLINT_SWI, RISCV_ACLINT_SWI_SIZE);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &swi->mmio);
swi->soft_irqs = g_malloc(sizeof(qemu_irq) * swi->num_harts);
qdev_init_gpio_out(dev, swi->soft_irqs, swi->num_harts);
+}
-/* Claim software interrupt bits */
-for (i = 0; i < swi->num_harts; i++) {
-RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(swi->hartid_base + i));
-/* We don't claim mip.SSIP because it is writeable by software */
-if (riscv_cpu_claim_interrupts(cpu, swi->sswi ? 0 : MIP_MSIP) < 0) {
-error_report("MSIP already claimed");
-exit(1);
-}
-}
+static void riscv_aclint_swi_finalize(Object *obj)
+{
+RISCVAclintSwiState *swi = RISCV_ACLINT_SWI(obj);
+
+/* free allocated area during realize */
+g_free(swi->soft_irqs);
}
static void riscv_aclint_swi_class_init(ObjectClass *klass, void *data)
@@ -415,10 +441,11 @@ static void riscv_aclint_swi_class_init(ObjectClass
*klass, void *data)
}
static const TypeInfo riscv_aclint_swi_info = {
-.name = TYPE_RISCV_ACLINT_SWI,
-.parent= TYPE_SYS_BUS_DEVICE,
-.instance_size = sizeof(RISCVAclintSwiState),
-.class_init= riscv_aclint_swi_class_init,
+.name = TYPE_RISCV_ACLINT_SWI,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(RISCVAclintSwiState),
+.class_init= riscv_aclint_swi_class_init,
+.instance_finalize = riscv_aclint_swi_finalize,
};
/*
@@ -430,7 +457,6 @@ DeviceState *riscv_aclint_swi_create(hwaddr addr, uint32_t
hartid_base,
int i;
DeviceState *dev = qdev_new(TYPE_RISCV_ACLINT_SWI);
-assert(num_harts <= RISCV_ACLINT_MAX_HARTS);
assert(!(addr & 0x3));
qdev_prop_set_uint32(dev, "hartid-base", hartid_base);
--
2.35.1
[PATCH 5/5] hw/intc/riscv_aclint: mtimer: report errors and free allocated memory
Add the instance_finalize() method to free the allocated arrays
and make realize() method report errors instead of exiting.
This commit also move the properties check from
riscv_aclint_mtimer_create() to realize().
code in realize() is re-ordered to first check for errors and leave
the object in a clean state. To achieve this, we do the following:
+ parse_hart_config and char_to_mode are refactored to return errors
instead of exiting.
+ in case of interrupt claim failure, we now release the succesfully
claimed ones.
These clean-ups allow the following life-cycle use cases (happening
when user creation is allowed) to execute as expected:
+ init -> finalize
+ init -> realize-failure -> finalize
Signed-off-by: Damien Hedde
---
hw/intc/riscv_aclint.c | 58 +++---
1 file changed, 43 insertions(+), 15 deletions(-)
diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index 4f66eeed0b..bef2e1988b 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -231,21 +231,51 @@ static void riscv_aclint_mtimer_realize(DeviceState *dev,
Error **errp)
RISCVAclintMTimerState *s = RISCV_ACLINT_MTIMER(dev);
int i;
+if (s->num_harts > RISCV_ACLINT_MAX_HARTS) {
+error_setg(errp, "invalid 'num-harts': max is %u",
+ RISCV_ACLINT_MAX_HARTS);
+return;
+}
+
+if (s->timecmp_base & 0x7) {
+error_setg(errp, "invalid 'timecmp-base': must be aligned on 0x8");
+return;
+}
+
+if (s->time_base & 0x7) {
+error_setg(errp, "invalid 'time-base': must be aligned on 0x8");
+return;
+}
+
+/* Claim timer interrupt bits */
+for (i = 0; i < s->num_harts; i++) {
+RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(s->hartid_base + i));
+if (riscv_cpu_claim_interrupts(cpu, MIP_MTIP) < 0) {
+error_setg(errp, "MTIP (hartid %u) already claimed",
+ (unsigned) (s->hartid_base + i));
+/* release interrupts we already claimed */
+while (--i >= 0) {
+cpu = RISCV_CPU(qemu_get_cpu(s->hartid_base + i));
+riscv_cpu_release_claimed_interrupts(cpu, MIP_MTIP);
+}
+return;
+}
+}
+
memory_region_init_io(&s->mmio, OBJECT(dev), &riscv_aclint_mtimer_ops,
s, TYPE_RISCV_ACLINT_MTIMER, s->aperture_size);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
s->timer_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts);
qdev_init_gpio_out(dev, s->timer_irqs, s->num_harts);
+}
-/* Claim timer interrupt bits */
-for (i = 0; i < s->num_harts; i++) {
-RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(s->hartid_base + i));
-if (riscv_cpu_claim_interrupts(cpu, MIP_MTIP) < 0) {
-error_report("MTIP already claimed");
-exit(1);
-}
-}
+static void riscv_aclint_mtimer_finalize(Object *obj)
+{
+RISCVAclintMTimerState *s = RISCV_ACLINT_MTIMER(obj);
+
+/* free allocated area during realize */
+g_free(s->timer_irqs);
}
static void riscv_aclint_mtimer_class_init(ObjectClass *klass, void *data)
@@ -256,10 +286,11 @@ static void riscv_aclint_mtimer_class_init(ObjectClass
*klass, void *data)
}
static const TypeInfo riscv_aclint_mtimer_info = {
-.name = TYPE_RISCV_ACLINT_MTIMER,
-.parent= TYPE_SYS_BUS_DEVICE,
-.instance_size = sizeof(RISCVAclintMTimerState),
-.class_init= riscv_aclint_mtimer_class_init,
+.name = TYPE_RISCV_ACLINT_MTIMER,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(RISCVAclintMTimerState),
+.class_init= riscv_aclint_mtimer_class_init,
+.instance_finalize = riscv_aclint_mtimer_finalize,
};
/*
@@ -273,10 +304,7 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr,
hwaddr size,
int i;
DeviceState *dev = qdev_new(TYPE_RISCV_ACLINT_MTIMER);
-assert(num_harts <= RISCV_ACLINT_MAX_HARTS);
assert(!(addr & 0x7));
-assert(!(timecmp_base & 0x7));
-assert(!(time_base & 0x7));
qdev_prop_set_uint32(dev, "hartid-base", hartid_base);
qdev_prop_set_uint32(dev, "num-harts", num_harts);
--
2.35.1
[PATCH 3/5] hw/intc/sifive_plic: report errors and free allocated memory
Add the instance_finalize() method to free the allocated arrays
and make realize() method report errors instead of exiting.
code in realize() is re-ordered to first check for errors and leave
the object in a clean state. To achieve this, we do the following:
+ parse_hart_config and char_to_mode are refactored to return errors
instead of exiting.
+ in case of interrupt claim failure, we now release the succesfully
claimed ones.
These clean-ups allow the following life-cycle use cases (happening
when user creation is allowed) to execute as expected:
+ init -> finalize
+ init -> realize-failure -> finalize
Signed-off-by: Damien Hedde
---
hw/intc/sifive_plic.c | 90 +--
1 file changed, 62 insertions(+), 28 deletions(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index eebbcf33d4..8692ea6725 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -37,16 +37,20 @@ static bool addr_between(uint32_t addr, uint32_t base,
uint32_t num)
return addr >= base && addr - base < num;
}
-static PLICMode char_to_mode(char c)
+static PLICMode char_to_mode(char c, bool *success)
{
+if (success) {
+*success = true;
+};
switch (c) {
case 'U': return PLICMode_U;
case 'S': return PLICMode_S;
case 'H': return PLICMode_H;
case 'M': return PLICMode_M;
default:
-error_report("plic: invalid mode '%c'", c);
-exit(1);
+g_assert(success != NULL);
+*success = false;
+return 0;
}
}
@@ -260,7 +264,7 @@ static void sifive_plic_reset(DeviceState *dev)
* "MS,MS" 2 harts, 0-1 with M and S mode
* "M,MS,MS,MS,MS" 5 harts, 0 with M mode, 1-5 with M and S mode
*/
-static void parse_hart_config(SiFivePLICState *plic)
+static bool parse_hart_config(SiFivePLICState *plic, Error **errp)
{
int addrid, hartid, modes;
const char *p;
@@ -275,11 +279,16 @@ static void parse_hart_config(SiFivePLICState *plic)
modes = 0;
hartid++;
} else {
-int m = 1 << char_to_mode(c);
+bool mode_ok = false;
+int m = 1 << char_to_mode(c, &mode_ok);
+if (!mode_ok) {
+error_setg(errp, "plic: invalid mode '%c'", c);
+return false;
+}
if (modes == (modes | m)) {
-error_report("plic: duplicate mode '%c' in config: %s",
- c, plic->hart_config);
-exit(1);
+error_setg(errp, "plic: duplicate mode '%c' in config: %s",
+ c, plic->hart_config);
+return false;
}
modes |= m;
}
@@ -302,10 +311,12 @@ static void parse_hart_config(SiFivePLICState *plic)
} else {
plic->addr_config[addrid].addrid = addrid;
plic->addr_config[addrid].hartid = hartid;
-plic->addr_config[addrid].mode = char_to_mode(c);
+plic->addr_config[addrid].mode = char_to_mode(c, NULL);
addrid++;
}
}
+
+return true;
}
static void sifive_plic_irq_request(void *opaque, int irq, int level)
@@ -321,12 +332,34 @@ static void sifive_plic_realize(DeviceState *dev, Error
**errp)
SiFivePLICState *s = SIFIVE_PLIC(dev);
int i;
+if (!parse_hart_config(s, errp)) {
+return;
+}
+
+/*
+ * We can't allow the supervisor to control SEIP as this would allow the
+ * supervisor to clear a pending external interrupt which will result in
+ * lost a interrupt in the case a PLIC is attached. The SEIP bit must be
+ * hardware controlled when a PLIC is attached.
+ */
+for (i = 0; i < s->num_harts; i++) {
+RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(s->hartid_base + i));
+if (riscv_cpu_claim_interrupts(cpu, MIP_SEIP) < 0) {
+error_setg(errp, "SEIP (hartid %u) already claimed",
+ (unsigned) (s->hartid_base + i));
+/* release interrupts we already claimed */
+while (--i >= 0) {
+cpu = RISCV_CPU(qemu_get_cpu(s->hartid_base + i));
+riscv_cpu_release_claimed_interrupts(cpu, MIP_SEIP);
+}
+return;
+}
+}
+
memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_plic_ops, s,
TYPE_SIFIVE_PLIC, s->aperture_size);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
-parse_hart_config(s);
-
s->bitfield_words = (s->num_sources + 31) >> 5;
s->num_enables = s->bitfield_words * s->num_addrs;
s->source_priority = g_new0(uint32_t, s->num_sources);
@@ -343,19 +376,6 @@ static void sifive_plic_realize(DeviceState *dev, Error
**errp)
s->m_external_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts);
qdev_init_gpio_out(dev, s->m_external_irqs, s->num_harts);
-/* We can't allow the supervisor to control SEIP as this would allow the
- * s
[PATCH 0/5] RiscV cleanups for user-related life cycles
Hi, This is a few cleanups regarding user oriented life-cycle use cases. When a device is accessible to user creation, there are a few life-cycle use cases to consider: + init -> finalize (happen when introspection the object). + init -> realize-failure -> finalize (realize must report errors due to miss configuration and leave in a 'good' state) This series fixes issues I've spotted in the riscv hart array and interrupt controllers. It is organized as follows: + patch 1 prevent memory leak in riscv array array + patch 2 introduce a new function in the riscv cpu needed by next pacthes + patches 3/4/5 prevent memory leaks and add error reporting in plic and aclint devices Thanks, -- Damien Damien Hedde (5): hw/riscv/riscv_hart: free the harts array when the object is finalized target/riscv: add riscv_cpu_release_claimed_interrupts function hw/intc/sifive_plic: report errors and free allocated memory hw/intc/riscv_aclint: swi: report errors and free allocated memory hw/intc/riscv_aclint: mtimer: report errors and free allocated memory target/riscv/cpu.h| 7 +++ hw/intc/riscv_aclint.c| 112 -- hw/intc/sifive_plic.c | 90 -- hw/riscv/riscv_hart.c | 8 +++ target/riscv/cpu_helper.c | 8 +++ 5 files changed, 168 insertions(+), 57 deletions(-) -- 2.35.1
[PATCH 2/5] target/riscv: add riscv_cpu_release_claimed_interrupts function
This function will be used to undo an interrupt claim made by
a previous call to riscv_cpu_claim_interrupts().
Signed-off-by: Damien Hedde
---
target/riscv/cpu.h| 7 +++
target/riscv/cpu_helper.c | 8
2 files changed, 15 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 8183fb86d5..9f0c432053 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -469,6 +469,13 @@ void riscv_cpu_list(void);
bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env);
int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts);
+
+/*
+ * riscv_cpu_release_unclaimed_interrupts:
+ * Release previously claimed interrupts by riscv_cpu_claim_interrupts().
+ */
+void riscv_cpu_release_claimed_interrupts(RISCVCPU *cpu, uint64_t interrupts);
+
uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value);
#define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(uint32_t),
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 746335bfd6..170fed6dff 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -596,6 +596,14 @@ int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t
interrupts)
}
}
+void riscv_cpu_release_claimed_interrupts(RISCVCPU *cpu, uint64_t interrupts)
+{
+CPURISCVState *env = &cpu->env;
+/* ensure all claimed interrupt are really there */
+g_assert((env->miclaim & interrupts) == interrupts);
+env->miclaim &= ~interrupts;
+}
+
uint64_t riscv_cpu_update_mip(RISCVCPU *cpu, uint64_t mask, uint64_t value)
{
CPURISCVState *env = &cpu->env;
--
2.35.1
[PATCH 1/5] hw/riscv/riscv_hart: free the harts array when the object is finalized
The array is dynamically allocated by realize() depending on the
number of harts.
This clean-up removes memory leaks which would happen in the
'init->finalize' life-cycle use-case (happening when user creation
is allowed).
Signed-off-by: Damien Hedde
---
hw/riscv/riscv_hart.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 613ea2aaa0..4aed6c2a59 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -66,6 +66,13 @@ static void riscv_harts_realize(DeviceState *dev, Error
**errp)
}
}
+static void riscv_harts_finalize(Object *obj)
+{
+RISCVHartArrayState *s = RISCV_HART_ARRAY(obj);
+
+g_free(s->harts);
+}
+
static void riscv_harts_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -79,6 +86,7 @@ static const TypeInfo riscv_harts_info = {
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(RISCVHartArrayState),
.class_init= riscv_harts_class_init,
+.instance_finalize = riscv_harts_finalize,
};
static void riscv_harts_register_types(void)
--
2.35.1
Re: [PATCH v8 3/5] migration: Add zero-copy-send parameter for QMP/HMP for Linux
Leonardo Bras wrote: > Add property that allows zero-copy migration of memory pages > on the sending side, and also includes a helper function > migrate_use_zero_copy_send() to check if it's enabled. > > No code is introduced to actually do the migration, but it allow > future implementations to enable/disable this feature. > > On non-Linux builds this parameter is compiled-out. > > Signed-off-by: Leonardo Bras > Reviewed-by: Peter Xu > Reviewed-by: Daniel P. Berrangé Reviewed-by: Juan Quintela
Re: [PATCH v8 2/5] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX
Leonardo Bras wrote: > For CONFIG_LINUX, implement the new zero copy flag and the optional callback > io_flush on QIOChannelSocket, but enables it only when MSG_ZEROCOPY > feature is available in the host kernel, which is checked on > qio_channel_socket_connect_sync() > > qio_channel_socket_flush() was implemented by counting how many times > sendmsg(...,MSG_ZEROCOPY) was successfully called, and then reading the > socket's error queue, in order to find how many of them finished sending. > Flush will loop until those counters are the same, or until some error occurs. > > Notes on using writev() with QIO_CHANNEL_WRITE_FLAG_ZERO_COPY: > 1: Buffer > - As MSG_ZEROCOPY tells the kernel to use the same user buffer to avoid > copying, > some caution is necessary to avoid overwriting any buffer before it's sent. > If something like this happen, a newer version of the buffer may be sent > instead. > - If this is a problem, it's recommended to call qio_channel_flush() before > freeing > or re-using the buffer. > > 2: Locked memory > - When using MSG_ZERCOCOPY, the buffer memory will be locked after queued, and > unlocked after it's sent. > - Depending on the size of each buffer, and how often it's sent, it may > require > a larger amount of locked memory than usually available to non-root user. > - If the required amount of locked memory is not available, writev_zero_copy > will return an error, which can abort an operation like migration, > - Because of this, when an user code wants to add zero copy as a feature, it > requires a mechanism to disable it, so it can still be accessible to less > privileged users. > > Signed-off-by: Leonardo Bras > Reviewed-by: Peter Xu > Reviewed-by: Daniel P. Berrangé Reviewed-by: Juan Quintela
Re: [PATCH v8 1/5] QIOChannel: Add flags on io_writev and introduce io_flush callback
Leonardo Bras wrote: > Add flags to io_writev and introduce io_flush as optional callback to > QIOChannelClass, allowing the implementation of zero copy writes by > subclasses. > > How to use them: > - Write data using qio_channel_writev*(...,QIO_CHANNEL_WRITE_FLAG_ZERO_COPY), > - Wait write completion with qio_channel_flush(). > > Notes: > As some zero copy write implementations work asynchronously, it's > recommended to keep the write buffer untouched until the return of > qio_channel_flush(), to avoid the risk of sending an updated buffer > instead of the buffer state during write. > > As io_flush callback is optional, if a subclass does not implement it, then: > - io_flush will return 0 without changing anything. > > Also, some functions like qio_channel_writev_full_all() were adapted to > receive a flag parameter. That allows shared code between zero copy and > non-zero copy writev, and also an easier implementation on new flags. > > Signed-off-by: Leonardo Bras Reviewed-by: Juan Quintela As everybody pointed out about the missing assertion...
Re: [PATCH v7 3/5] migration: Add zero-copy parameter for QMP/HMP for Linux
Leonardo Bras Soares Passos wrote: > On Wed, Jan 19, 2022 at 3:16 PM Daniel P. Berrangé > wrote: >> >> On Wed, Jan 19, 2022 at 03:03:29PM -0300, Leonardo Bras Soares Passos wrote: >> > Hello Daniel, >> > >> > On Thu, Jan 13, 2022 at 10:10 AM Daniel P. Berrangé >> > wrote: >> > > >> > > On Thu, Jan 06, 2022 at 07:13:40PM -0300, Leonardo Bras wrote: >> > > > Add property that allows zero-copy migration of memory pages, >> > > > and also includes a helper function migrate_use_zero_copy() to check >> > > > if it's enabled. >> > > > >> > > > No code is introduced to actually do the migration, but it allow >> > > > future implementations to enable/disable this feature. >> > > > >> > > > On non-Linux builds this parameter is compiled-out. >> > > > >> > > > Signed-off-by: Leonardo Bras >> > > > --- >> > > > qapi/migration.json | 24 >> > > > migration/migration.h | 5 + >> > > > migration/migration.c | 32 >> > > > migration/socket.c| 5 + >> > > > monitor/hmp-cmds.c| 6 ++ >> > > > 5 files changed, 72 insertions(+) >> > > >> > > Reviewed-by: Daniel P. Berrangé >> > >> > Thanks! >> > > Ok, I see the point. > I will try to refactor the code changing zero-copy to zero-copy-send > or something like that. Hi I am late to the party, but I agree with Dan that we need two flags. Thre reason is that you can be the target of one migration, and later be the source of a next one. If we only have one flag that means different things on the source and destination side, things become really complicated. Later, Juan.
Re: Call for GSoC and Outreachy project ideas for summer 2022
On 2/18/22 12:39, Michal Prívozník wrote: On 2/17/22 18:52, Paolo Bonzini wrote: I would like to co-mentor one or more projects about adding more statistics to Mark Kanda's newly-born introspectable statistics subsystem in QEMU (https://patchew.org/QEMU/20220215150433.2310711-1-mark.ka...@oracle.com/), for example integrating "info blockstats"; and/or, to add matching functionality to libvirt. However, I will only be available for co-mentoring unfortunately. I'm happy to offer my helping hand in this. I mean the libvirt part, since I am a libvirt developer. I believe this will be listed in QEMU's ideas list, right? Does Libvirt participate to GSoC as an independent organization this year? If not, I'll add it as a Libvirt project on the QEMU ideas list. Paolo
Re: [PATCH] build: fix build failure with gcc 11.2 by disabling -fcf-protection
On 2/8/22 22:19, Vineet Gupta wrote: When doing RV qemu builds with host gcc 11.2, ran into following build failure | cc -MMD -MP -MT linuxboot_dma.o -MF ./linuxboot_dma.d -O2 -g -march=i486 -Wall \ | -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes \ | -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security \ | -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs \ | -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs \ | -Wno-shift-negative-value -Wno-psabi -fno-pie -ffreestanding -IQEMU/include \ | -fno-stack-protector -m16 -Wa,-32 \ | -c QEMU/pc-bios/optionrom/linuxboot_dma.c -o linuxboot_dma.o |cc1: error: ‘-fcf-protection’ is not compatible with this target Signed-off-by: Vineet Gupta --- This might be a crude fix to the problem --- pc-bios/optionrom/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile index 5d55d25acca2..8f843ee803c1 100644 --- a/pc-bios/optionrom/Makefile +++ b/pc-bios/optionrom/Makefile @@ -22,6 +22,9 @@ override CFLAGS += $(CFLAGS_NOPIE) -ffreestanding -I$(TOPSRC_DIR)/include override CFLAGS += $(call cc-option, -fno-stack-protector) override CFLAGS += $(call cc-option, -m16) +# issue with gcc 11.2 +override CFLAGS += $(call cc-option, -fcf-protection=none) + ifeq ($(filter -m16, $(CFLAGS)),) # Attempt to work around compilers that lack -m16 (GCC <= 4.8, clang <= ??) # On GCC we add -fno-toplevel-reorder to keep the order of asm blocks with This is a problem in the Ubuntu compiler that you're using. If -fcf-protection is not compatible with -m16, Ubuntu should not enable it instead of breaking -m16. Besides, QEMU is not at all compatible with -fcf-protection, not just the ROMs. So it should be added to QEMU_CFLAGS in configure as well. Paolo
Re: [PATCH v2 6/8] configure: Pass filtered QEMU_OBJCFLAGS to meson
On 2/15/22 18:01, Philippe Mathieu-Daudé via wrote:
Filter unsupported Objective-C options, to avoid
'unknown-warning-option' warnings when using Clang:
[34/373] Compiling Objective-C object libcommon.fa.p/audio_coreaudio.m.o
warning: unknown warning option '-Wold-style-declaration'; did you mean
'-Wout-of-line-declaration'? [-Wunknown-warning-option]
warning: unknown warning option '-Wimplicit-fallthrough=2'; did you mean
'-Wimplicit-fallthrough'? [-Wunknown-warning-option]
2 warnings generated.
Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Paolo Bonzini
---
configure | 23 +++
meson.build | 5 -
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 3217aa22cb..df6eaec067 100755
--- a/configure
+++ b/configure
@@ -77,6 +77,7 @@ TMPB="qemu-conf"
TMPC="${TMPDIR1}/${TMPB}.c"
TMPO="${TMPDIR1}/${TMPB}.o"
TMPCXX="${TMPDIR1}/${TMPB}.cxx"
+TMPM="${TMPDIR1}/${TMPB}.m"
TMPE="${TMPDIR1}/${TMPB}.exe"
rm -f config.log
@@ -148,6 +149,10 @@ do_cxx() {
do_compiler "$cxx" $CPU_CFLAGS "$@"
}
+do_objc() {
+do_compiler "$objcc" $CPU_CFLAGS "$@"
+}
+
# Append $2 to the variable named $1, with space separation
add_to() {
eval $1=\${$1:+\"\$$1 \"}\$2
@@ -1616,10 +1621,27 @@ cc_has_warning_flag() {
compile_prog "-Werror $optflag" ""
}
+objcc_has_warning_flag() {
+cat > $TMPM <
if test "$stack_protector" != "no"; then
@@ -3579,6 +3601,7 @@ echo "LD=$ld" >> $config_host_mak
echo "CFLAGS_NOPIE=$CFLAGS_NOPIE" >> $config_host_mak
echo "QEMU_CFLAGS=$QEMU_CFLAGS" >> $config_host_mak
echo "QEMU_CXXFLAGS=$QEMU_CXXFLAGS" >> $config_host_mak
+echo "QEMU_OBJCFLAGS=$QEMU_OBJCFLAGS" >> $config_host_mak
echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
echo "GLIB_LIBS=$glib_libs" >> $config_host_mak
echo "GLIB_VERSION=$(pkg-config --modversion glib-2.0)" >> $config_host_mak
diff --git a/meson.build b/meson.build
index 215c253683..7b78235a39 100644
--- a/meson.build
+++ b/meson.build
@@ -199,9 +199,11 @@ if get_option('fuzzing')
endif
add_global_arguments(config_host['QEMU_CFLAGS'].split(),
- native: false, language: ['c', 'objc'])
+ native: false, language: ['c'])
add_global_arguments(config_host['QEMU_CXXFLAGS'].split(),
native: false, language: 'cpp')
+add_global_arguments(config_host['QEMU_OBJCFLAGS'].split(),
+ native: false, language: ['objc'])
add_global_link_arguments(config_host['QEMU_LDFLAGS'].split(),
native: false, language: ['c', 'cpp', 'objc'])
@@ -3306,6 +3308,7 @@ if link_args.length() > 0
endif
summary_info += {'QEMU_CFLAGS': config_host['QEMU_CFLAGS']}
summary_info += {'QEMU_CXXFLAGS': config_host['QEMU_CXXFLAGS']}
+summary_info += {'QEMU_OBJCFLAGS':config_host['QEMU_OBJCFLAGS']}
summary_info += {'QEMU_LDFLAGS': config_host['QEMU_LDFLAGS']}
summary_info += {'profiler': config_host.has_key('CONFIG_PROFILER')}
summary_info += {'link-time optimization (LTO)': get_option('b_lto')}
Re: [PATCH v2 4/8] configure: Disable out-of-line atomic operations on Aarch64
On 2/15/22 18:01, Philippe Mathieu-Daudé via wrote: + +case "$cpu" in + aarch64) +write_c_skeleton; +if compile_prog "$CPU_CFLAGS -Werror -mno-outline-atomics" "" ; then + CPU_CFLAGS="-mno-outline-atomics $CPU_CFLAGS" +fi +;; Apart from the question of whether/how to work around this issue, this should not be added to CPU_CFLAGS. CPU_CFLAGS is only for things that change the ABI. Paolo
Re: [PATCH v2 4/8] configure: Disable out-of-line atomic operations on Aarch64
On 2/18/22 02:46, Richard Henderson wrote: I don't have gobjc/g++ installed, so ./configure defaulted to Clang to compile these languages, but compiled C files using GCC. At the end the Clang linker is used (the default c++ symlink). This is another form of compiler mis-configuration. If you don't have g++ to go with gcc, use --cxx=false to avoid picking up a different compiler. This would be the kind of problem that this test is trying to cover: if do_cxx $CXXFLAGS $EXTRA_CXXFLAGS $CONFIGURE_CXXFLAGS $QEMU_CXXFLAGS -o $TMPE $TMPCXX $TMPO $QEMU_LDFLAGS; then # C++ compiler $cxx works ok with C compiler $cc : else echo "C++ compiler $cxx does not work with C compiler $cc" echo "Disabling C++ specific optional code" cxx= fi In the past it detected issues with libasan/libtsan incompatibilities. We should either add a test for atomic operations, or just drop the test. Paolo
Re: [PATCH v5 00/16] host: Support macOS 12
On Mon, 14 Feb 2022 at 18:56, Philippe Mathieu-Daudé wrote: > > Few patches to be able to build QEMU on macOS 12 (Monterey). > > This basically consists of adapting deprecated APIs. > > CI job added to avoid bitrotting. Hi; I'm going to take the "obviously correct (to me)" cocoa patches from here via target-arm.next: * MAINTAINERS patch * ui/cocoa: Remove allowedFileTypes restriction in SavePanel * ui/cocoa: Do not alert even without block devices * ui/cocoa: Fix the leak of qemu_console_get_label Let me know if that's awkward for you and you'd rather I dropped them. thanks -- PMM
Re: [PATCH v6 01/19] configure, meson: override C compiler for cmake
> On Feb 18, 2022, at 9:49 AM, Jag Raman wrote: > > > >> On Feb 18, 2022, at 7:13 AM, Paolo Bonzini wrote: >> >> On 2/18/22 04:40, Jag Raman wrote: On Feb 17, 2022, at 7:09 AM, Peter Maydell wrote: On Thu, 17 Feb 2022 at 07:56, Jagannathan Raman wrote: > > The compiler path that cmake gets from meson is corrupted. It results in > the following error: > | -- The C compiler identification is unknown > | CMake Error at CMakeLists.txt:35 (project): > | The CMAKE_C_COMPILER: > | /opt/rh/devtoolset-9/root/bin/cc;-m64;-mcx16 > | is not a full path to an existing compiler tool. > > Explicitly specify the C compiler for cmake to avoid this error This sounds like a bug in Meson. Is there a Meson bug report we can reference in the commit message here ? >>> Hi Peter, >>> This issue reproduces with the latest meson [1] also. >> >> 0.60.0 or more recent versions should have a fix, which would do exactly >> what this patch does: do not define CMAKE_C_COMPILER_LAUNCHER, and place the >> whole binaries.c variable in CMAKE_C_COMPILER. What are the contents of the >> genrated CMakeMesonToolchainFile.cmake and CMakeCache.txt files, without and >> with your patch? > > I’ll checkout what’s going on at my end. But the issue reproduces with > meson 0.61 from what I can tell: > # ../configure --target-list=x86_64-softmmu --enable-debug > --enable-vfio-user-server; > The Meson build system > Version: 0.61.2 > … > … > | /opt/rh/devtoolset-9/root/usr/bin/cc;-m64;-mcx16 > > | is not a full path to an existing compiler tool. > > > 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’m not sure if you’re interested in the contents of the whole file. But they’re here: Without patch: https://pastebin.com/sbwtvHy0 (also has error log at the end) With patch: https://pastebin.com/buRYSp2R Thank you! -- Jag > >> >>> I noticed the following about the “binaries” section [2]. The manual >>> says meson could pass the values in this section to find_program [3]. >>> As such I’m wondering if it’s OK to set compiler flags in this section >>> because find_program doesn’t seem to accept any compiler flags. >> >> The full quote of the manual is "These can be used internally by Meson, or >> by the find_program function", and the C compiler variable "c" is in the >> former category. >> >> There is an important difference between the flags in "binaries" and those >> in "built-in options". What is in "binaries" is used when requesting e.g. >> the compiler search path, while what is in "built-in options" is not. So >> options like "-m32" are definitely part of "binaries", not "built-in >> options": >> >> $ gcc --print-multi-os-directory >> ../lib64 >> $ gcc -m32 --print-multi-os-directory >> ../lib > > Do you know if the “host_machine” section in cross build > definition file [1] would be any help here? > > [1]: https://mesonbuild.com/Cross-compilation.html#machine-entries > > -- > Jag > >> >> Paolo
Re: [PATCH] hw/arm: add initial mori-bmc board
On Tue, 8 Feb 2022 at 23:31, Patrick Venture wrote: > > This is the BMC attached to the OpenBMC Mori board. > > Signed-off-by: Patrick Venture > Reviewed-by: Chris Rauer > Reviewed-by: Ilkyun Choi > --- > docs/system/arm/nuvoton.rst | 1 + > hw/arm/npcm7xx_boards.c | 32 > 2 files changed, 33 insertions(+) Applied to target-arm.next, thanks. -- PMM
Re: [PATCH v2] scripts/qapi: minor delinting
Markus Armbruster writes: > John Snow writes: > >> Get isort and pylint tools passing again. >> >> Signed-off-by: John Snow >> --- >> scripts/qapi/commands.py | 2 +- >> scripts/qapi/pylintrc| 15 +-- >> scripts/qapi/types.py| 6 +- >> scripts/qapi/visit.py| 6 +- >> 4 files changed, 16 insertions(+), 13 deletions(-) >> >> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py >> index 869d799ed22..38ca38a7b9d 100644 >> --- a/scripts/qapi/commands.py >> +++ b/scripts/qapi/commands.py >> @@ -25,8 +25,8 @@ >> QAPIGenC, >> QAPISchemaModularCVisitor, >> build_params, >> -ifcontext, >> gen_special_features, >> +ifcontext, >> ) >> from .schema import ( >> QAPISchema, >> diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc >> index b259531a726..1fed2e69620 100644 >> --- a/scripts/qapi/pylintrc >> +++ b/scripts/qapi/pylintrc >> @@ -34,16 +34,11 @@ disable=fixme, >> >> [BASIC] >> >> -# Good variable names which should always be accepted, separated by a comma. >> -good-names=i, >> - j, >> - k, >> - ex, >> - Run, >> - _, >> - fp, # fp = open(...) >> - fd, # fd = os.open(...) >> - ch, >> +# Good variable names regexes, separated by a comma. If names match any >> regex, >> +# they will always be accepted >> + >> +# Allow just about anything, as per Markus's preference. > > Does it still flag PEP-8 violations like all lower case class names? Looks like it in my tests. > If yes, "just about any length" is more precise. I'll change the comment if you don't mind: # Suppress complaints about short names. PEP-8 is cool with them, # and so are we. With that: Reviewed-by: Markus Armbruster No respin necessary.
'make check-acceptance' failing on s390 tests?
Hi; is anybody else seeing 'make check-acceptance' fail on some of
the s390 tests?
(009/183) tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred:
Timeout reached\nOriginal status: ERROR\n{'name':
'009-tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg',
'logdir':
'/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/tests/results/j...
(900.20 s)
(090/183)
tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora:
FAIL: b'1280 800\n' != b'1024 768\n' (26.79 s)
I've cc'd Daniel because the 090 at least looks like a resolution
baked into the test case, and commit de72c4b7c that went in
last month changed the EDID reported resolution from 1024x768
to 1280x800.
Not sure about the timeout on the boot test: the avocado log
shows it booting at least as far as
"Kernel 5.3.7-301.fc31.s390x on an s390x (ttysclp0)"
and then there's no further output until the timeout.
Unfortunately the avocado log doesn't seem to include useful
information like "this is the string we were waiting to see", so
I'm not sure exactly what's gone wrong there.
(I continue to find the Avocado tests rather opaque: when you
get a series of green OK's that's fine, but when you get a failure
it's often non-obvious why it failed or how to do simple things
like "rerun just that one failed test" or "run the failing command,
interactively on the command line".)
The 090 failure didn't cause the merge to be rejected because
in commit 333168efe5c8 we disabled both these tests when
running on GitLab.
Suggestion: we should either disable tests entirely (except
for manual "I want to run this known-flaky test") or not at
all, rather than disabling them only on GitLab. If I'm running
'make check-acceptance' locally I don't want to be distracted
by tests we know to be dodgy, any more than if I were running
the CI on GitLab.
thanks
-- PMM
[PATCH 1/2] keyval: Fix grammar comment to cover downstream prefix
According to the grammar, a key __com.redhat_foo would be parsed as
two key fragments __com and redhat_foo. It's actually parsed as a
single fragment. Fix the grammar.
Signed-off-by: Markus Armbruster
---
util/keyval.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/util/keyval.c b/util/keyval.c
index 904337c8a1..0cf2e84dc8 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -16,7 +16,9 @@
* key-vals = [ key-val { ',' key-val } [ ',' ] ]
* key-val = key '=' val | help
* key = key-fragment { '.' key-fragment }
- * key-fragment = / [^=,.]+ /
+ * key-fragment = qapi-name | index
+ * qapi-name= '__' / [a-z0-9.-]+ / '_' / [A-Za-z][A-Za-z0-9_-]* /
+ * index= / [0-9]+ /
* val = { / [^,]+ / | ',,' }
* help = 'help' | '?'
*
--
2.35.1
[PATCH 2/2] qapi: Fix stale reference to scripts/qapi.py in a comment
Signed-off-by: Markus Armbruster --- qapi/qapi-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c index fda7044539..63596e11c5 100644 --- a/qapi/qapi-util.c +++ b/qapi/qapi-util.c @@ -113,7 +113,7 @@ bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error **err * may contain only letters, digits, hyphen and period. * The special exception for enumeration names is not implemented. * See docs/devel/qapi-code-gen.txt for more on QAPI naming rules. - * Keep this consistent with scripts/qapi.py! + * Keep this consistent with scripts/qapi-gen.py! * If @complete, the parse fails unless it consumes @str completely. * Return its length on success, -1 on failure. */ -- 2.35.1
Re: [PATCH v6 17/19] vfio-user: register handlers to facilitate migration
> On Feb 18, 2022, at 7:20 AM, Paolo Bonzini wrote:
>
> On 2/17/22 08:49, Jagannathan Raman wrote:
>> Store and load the device's state during migration. use libvfio-user's
>> handlers for this purpose
>> Signed-off-by: Elena Ufimtseva
>> Signed-off-by: John G Johnson
>> Signed-off-by: Jagannathan Raman
>
> Why does no one call clear_deferred_backend_init?
We’ll clear it at the machine finalization. FWIW, the ‘x-remote’ machine
operates in a deferred backend initialization mode for the entire
lifecycle of the VM.
Thank you Paolo!
--
Jag
>
> Paolo
>
>> ---
>> include/block/block.h | 1 +
>> include/migration/vmstate.h | 2 +
>> migration/savevm.h | 2 +
>> block.c | 5 +
>> hw/remote/machine.c | 7 +
>> hw/remote/vfio-user-obj.c | 467
>> migration/savevm.c | 89 +++
>> migration/vmstate.c | 19 ++
>> 8 files changed, 592 insertions(+)
>> diff --git a/include/block/block.h b/include/block/block.h
>> index e1713ee306..02b89e0668 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -495,6 +495,7 @@ int generated_co_wrapper
>> bdrv_invalidate_cache(BlockDriverState *bs,
>> Error **errp);
>> void bdrv_invalidate_cache_all(Error **errp);
>> int bdrv_inactivate_all(void);
>> +int bdrv_inactivate(BlockDriverState *bs);
>>/* Ensure contents are flushed to disk. */
>> int generated_co_wrapper bdrv_flush(BlockDriverState *bs);
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index 017c03675c..68bea576ea 100644
>> --- a/include/migration/vmstate.h
>> +++ b/include/migration/vmstate.h
>> @@ -1165,6 +1165,8 @@ extern const VMStateInfo vmstate_info_qlist;
>> #define VMSTATE_END_OF_LIST() \
>> {}
>> +uint64_t vmstate_vmsd_size(PCIDevice *pci_dev);
>> +
>> int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>> void *opaque, int version_id);
>> int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>> diff --git a/migration/savevm.h b/migration/savevm.h
>> index 6461342cb4..8007064ff2 100644
>> --- a/migration/savevm.h
>> +++ b/migration/savevm.h
>> @@ -67,5 +67,7 @@ int qemu_loadvm_state_main(QEMUFile *f,
>> MigrationIncomingState *mis);
>> int qemu_load_device_state(QEMUFile *f);
>> int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>> bool in_postcopy, bool inactivate_disks);
>> +int qemu_remote_savevm(QEMUFile *f, DeviceState *dev);
>> +int qemu_remote_loadvm(QEMUFile *f);
>>#endif
>> diff --git a/block.c b/block.c
>> index b54d59d1fa..e90aaee30c 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -6565,6 +6565,11 @@ static int bdrv_inactivate_recurse(BlockDriverState
>> *bs)
>> return 0;
>> }
>> +int bdrv_inactivate(BlockDriverState *bs)
>> +{
>> +return bdrv_inactivate_recurse(bs);
>> +}
>> +
>> int bdrv_inactivate_all(void)
>> {
>> BlockDriverState *bs = NULL;
>> diff --git a/hw/remote/machine.c b/hw/remote/machine.c
>> index a8b4a3aef3..31ef401e43 100644
>> --- a/hw/remote/machine.c
>> +++ b/hw/remote/machine.c
>> @@ -24,6 +24,7 @@
>> #include "hw/qdev-core.h"
>> #include "hw/remote/iommu.h"
>> #include "hw/remote/vfio-user-obj.h"
>> +#include "sysemu/sysemu.h"
>>static void remote_machine_init(MachineState *machine)
>> {
>> @@ -86,6 +87,11 @@ static void remote_machine_set_vfio_user(Object *obj,
>> bool value, Error **errp)
>> s->vfio_user = value;
>> }
>> +static void remote_machine_instance_init(Object *obj)
>> +{
>> +set_deferred_backend_init();
>> +}
>> +
>> static void remote_machine_class_init(ObjectClass *oc, void *data)
>> {
>> MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -105,6 +111,7 @@ static const TypeInfo remote_machine = {
>> .name = TYPE_REMOTE_MACHINE,
>> .parent = TYPE_MACHINE,
>> .instance_size = sizeof(RemoteMachineState),
>> +.instance_init = remote_machine_instance_init,
>> .class_init = remote_machine_class_init,
>> .interfaces = (InterfaceInfo[]) {
>> { TYPE_HOTPLUG_HANDLER },
>> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
>> index d79bab87f1..2304643003 100644
>> --- a/hw/remote/vfio-user-obj.c
>> +++ b/hw/remote/vfio-user-obj.c
>> @@ -57,6 +57,13 @@
>> #include "hw/pci/msi.h"
>> #include "hw/pci/msix.h"
>> #include "hw/remote/vfio-user-obj.h"
>> +#include "migration/qemu-file.h"
>> +#include "migration/savevm.h"
>> +#include "migration/vmstate.h"
>> +#include "migration/global_state.h"
>> +#include "block/block.h"
>> +#include "sysemu/block-backend.h"
>> +#include "net/net.h"
>>#define TYPE_VFU_OBJECT "x-vfio-user-server"
>> OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT)
>> @@ -108,12 +115,49 @@ struct VfuObject {
>> Error *unplug_blocker;
>>int vfu_poll_fd;
>> +
>> +/*
>> + * vfu_mig_buf h
[PATCH 0/2] Minor doc fixes
Markus Armbruster (2): keyval: Fix grammar comment to cover downstream prefix qapi: Fix stale reference to scripts/qapi.py in a comment qapi/qapi-util.c | 2 +- util/keyval.c| 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) -- 2.35.1
Re: [PATCH v6 01/19] configure, meson: override C compiler for cmake
> On Feb 18, 2022, at 7:13 AM, Paolo Bonzini wrote: > > On 2/18/22 04:40, Jag Raman wrote: >>> On Feb 17, 2022, at 7:09 AM, Peter Maydell wrote: >>> >>> On Thu, 17 Feb 2022 at 07:56, Jagannathan Raman >>> wrote: The compiler path that cmake gets from meson is corrupted. It results in the following error: | -- The C compiler identification is unknown | CMake Error at CMakeLists.txt:35 (project): | The CMAKE_C_COMPILER: | /opt/rh/devtoolset-9/root/bin/cc;-m64;-mcx16 | is not a full path to an existing compiler tool. Explicitly specify the C compiler for cmake to avoid this error >>> >>> This sounds like a bug in Meson. Is there a Meson bug report >>> we can reference in the commit message here ? >> Hi Peter, >> This issue reproduces with the latest meson [1] also. > > 0.60.0 or more recent versions should have a fix, which would do exactly what > this patch does: do not define CMAKE_C_COMPILER_LAUNCHER, and place the whole > binaries.c variable in CMAKE_C_COMPILER. What are the contents of the > genrated CMakeMesonToolchainFile.cmake and CMakeCache.txt files, without and > with your patch? I’ll checkout what’s going on at my end. But the issue reproduces with meson 0.61 from what I can tell: # ../configure --target-list=x86_64-softmmu --enable-debug --enable-vfio-user-server; The Meson build system Version: 0.61.2 … … | /opt/rh/devtoolset-9/root/usr/bin/cc;-m64;-mcx16 | is not a full path to an existing compiler tool. 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 noticed the following about the “binaries” section [2]. The manual >> says meson could pass the values in this section to find_program [3]. >> As such I’m wondering if it’s OK to set compiler flags in this section >> because find_program doesn’t seem to accept any compiler flags. > > The full quote of the manual is "These can be used internally by Meson, or by > the find_program function", and the C compiler variable "c" is in the former > category. > > There is an important difference between the flags in "binaries" and those in > "built-in options". What is in "binaries" is used when requesting e.g. the > compiler search path, while what is in "built-in options" is not. So options > like "-m32" are definitely part of "binaries", not "built-in options": > >$ gcc --print-multi-os-directory >../lib64 >$ gcc -m32 --print-multi-os-directory >../lib Do you know if the “host_machine” section in cross build definition file [1] would be any help here? [1]: https://mesonbuild.com/Cross-compilation.html#machine-entries -- Jag > > Paolo
Re: [PATCH v5 12/15] hw/nvme: Initialize capability structures for primary/secondary controllers
On Thu, Feb 17, 2022 at 06:45:01PM +0100, Lukasz Maniak wrote:
> From: Łukasz Gieryk
>
> With four new properties:
> - sriov_v{i,q}_flexible,
> - sriov_max_v{i,q}_per_vf,
> one can configure the number of available flexible resources, as well as
> the limits. The primary and secondary controller capability structures
> are initialized accordingly.
>
> Since the number of available queues (interrupts) now varies between
> VF/PF, BAR size calculation is also adjusted.
>
> Signed-off-by: Łukasz Gieryk
> ---
> hw/nvme/ctrl.c | 142 ---
> hw/nvme/nvme.h | 4 ++
> include/block/nvme.h | 5 ++
> 3 files changed, 144 insertions(+), 7 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 73707565345..2a6a36e733d 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -36,6 +36,10 @@
> * zoned.zasl=, \
> * zoned.auto_transition=, \
> * sriov_max_vfs= \
> + * sriov_vq_flexible= \
> + * sriov_vi_flexible= \
> + * sriov_max_vi_per_vf= \
> + * sriov_max_vq_per_vf= \
> * subsys=
> * -device nvme-ns,drive=,bus=,nsid=,\
> * zoned=, \
> @@ -113,6 +117,29 @@
> * enables reporting of both SR-IOV and ARI capabilities by the NVMe
> device.
> * Virtual function controllers will not report SR-IOV capability.
> *
> + * NOTE: Single Root I/O Virtualization support is experimental.
> + * All the related parameters may be subject to change.
> + *
> + * - `sriov_vq_flexible`
> + * Indicates the total number of flexible queue resources assignable to all
> + * the secondary controllers. Implicitly sets the number of primary
> + * controller's private resources to `(max_ioqpairs - sriov_vq_flexible)`.
> + *
> + * - `sriov_vi_flexible`
> + * Indicates the total number of flexible interrupt resources assignable to
> + * all the secondary controllers. Implicitly sets the number of primary
> + * controller's private resources to `(msix_qsize - sriov_vi_flexible)`.
> + *
> + * - `sriov_max_vi_per_vf`
> + * Indicates the maximum number of virtual interrupt resources assignable
> + * to a secondary controller. The default 0 resolves to
> + * `(sriov_vi_flexible / sriov_max_vfs)`.
> + *
> + * - `sriov_max_vq_per_vf`
> + * Indicates the maximum number of virtual queue resources assignable to
> + * a secondary controller. The default 0 resolves to
> + * `(sriov_vq_flexible / sriov_max_vfs)`.
> + *
> * nvme namespace device parameters
> *
> * - `shared`
> @@ -184,6 +211,7 @@
> #define NVME_NUM_FW_SLOTS 1
> #define NVME_DEFAULT_MAX_ZA_SIZE (128 * KiB)
> #define NVME_MAX_VFS 127
> +#define NVME_VF_RES_GRANULARITY 1
> #define NVME_VF_OFFSET 0x1
> #define NVME_VF_STRIDE 1
>
> @@ -6512,6 +6540,54 @@ static void nvme_check_constraints(NvmeCtrl *n, Error
> **errp)
> error_setg(errp, "PMR is not supported with SR-IOV");
> return;
> }
> +
> +if (!params->sriov_vq_flexible || !params->sriov_vi_flexible) {
> +error_setg(errp, "both sriov_vq_flexible and sriov_vi_flexible"
> + " must be set for the use of SR-IOV");
> +return;
> +}
> +
> +if (params->sriov_vq_flexible < params->sriov_max_vfs * 2) {
> +error_setg(errp, "sriov_vq_flexible must be greater than or
> equal"
> + " to %d (sriov_max_vfs * 2)", params->sriov_max_vfs *
> 2);
> +return;
> +}
> +
> +if (params->max_ioqpairs < params->sriov_vq_flexible + 2) {
> +error_setg(errp, "sriov_vq_flexible - max_ioqpairs (PF-private"
After posting, we realized that the error string is confusing. This will
be fixed in v6.
> + " queue resources) must be greater than or equal to
> 2");
> +return;
> +}
> +
> +if (params->sriov_vi_flexible < params->sriov_max_vfs) {
> +error_setg(errp, "sriov_vi_flexible must be greater than or
> equal"
> + " to %d (sriov_max_vfs)", params->sriov_max_vfs);
> +return;
> +}
> +
> +if (params->msix_qsize < params->sriov_vi_flexible + 1) {
> +error_setg(errp, "sriov_vi_flexible - msix_qsize (PF-private"
Same here.
> + " interrupt resources) must be greater than or equal"
> + " to 1");
> +return;
> +}
> +
> +if (params->sriov_max_vi_per_vf &&
> +(params->sriov_max_vi_per_vf - 1) % NVME_VF_RES_GRANULARITY) {
> +error_setg(errp, "sriov_max_vi_per_vf must meet:"
> + " (X - 1) %% %d == 0 and X >= 1",
> + NVME_VF_RES_GRANULARITY);
> +return;
> +}
> +
> +if (params->sriov_max_vq_per_vf &&
> +(params->sriov_max_vq_per_vf <
Re: [PATCH v5 00/15] hw/nvme: SR-IOV with Virtualization Enhancements
On Fri, Feb 18, 2022 at 03:23:15AM -0500, Michael S. Tsirkin wrote: > On Thu, Feb 17, 2022 at 06:44:49PM +0100, Lukasz Maniak wrote: > > Changes since v4: > > - Added hello world example for SR-IOV to the docs > > - Moved AER initialization from nvme_init_ctrl to nvme_init_state > > - Fixed division by zero issue in calculation of vqfrt and vifrt > > capabilities > > > BTW you should copy all reviewers on the cover letter. > Yep, will do next time. Sorry about that. > > > > Knut Omang (2): > > pcie: Add support for Single Root I/O Virtualization (SR/IOV) > > pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt > > > > Lukasz Maniak (4): > > hw/nvme: Add support for SR-IOV > > hw/nvme: Add support for Primary Controller Capabilities > > hw/nvme: Add support for Secondary Controller List > > docs: Add documentation for SR-IOV and Virtualization Enhancements > > > > Łukasz Gieryk (9): > > pcie: Add a helper to the SR/IOV API > > pcie: Add 1.2 version token for the Power Management Capability > > hw/nvme: Implement the Function Level Reset > > hw/nvme: Make max_ioqpairs and msix_qsize configurable in runtime > > hw/nvme: Remove reg_size variable and update BAR0 size calculation > > hw/nvme: Calculate BAR attributes in a function > > hw/nvme: Initialize capability structures for primary/secondary > > controllers > > hw/nvme: Add support for the Virtualization Management command > > hw/nvme: Update the initalization place for the AER queue > > > > docs/pcie_sriov.txt | 115 ++ > > docs/system/devices/nvme.rst | 82 + > > hw/nvme/ctrl.c | 674 --- > > hw/nvme/ns.c | 2 +- > > hw/nvme/nvme.h | 55 ++- > > hw/nvme/subsys.c | 75 +++- > > hw/nvme/trace-events | 6 + > > hw/pci/meson.build | 1 + > > hw/pci/pci.c | 100 -- > > hw/pci/pcie.c| 5 + > > hw/pci/pcie_sriov.c | 302 > > hw/pci/trace-events | 5 + > > include/block/nvme.h | 65 > > include/hw/pci/pci.h | 12 +- > > include/hw/pci/pci_ids.h | 1 + > > include/hw/pci/pci_regs.h| 1 + > > include/hw/pci/pcie.h| 6 + > > include/hw/pci/pcie_sriov.h | 77 > > include/qemu/typedefs.h | 2 + > > 19 files changed, 1505 insertions(+), 81 deletions(-) > > create mode 100644 docs/pcie_sriov.txt > > create mode 100644 hw/pci/pcie_sriov.c > > create mode 100644 include/hw/pci/pcie_sriov.h > > > > -- > > 2.25.1 > > > > > > >
Re: [PATCH v2 3/6] hw/misc: Add a model of the Xilinx ZynqMP CRF
On Thu, 3 Feb 2022 at 14:01, Edgar E. Iglesias wrote: > > From: "Edgar E. Iglesias" > > Add a model of the Xilinx ZynqMP CRF. At the moment this > is mostly a stub model. > > Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: Edgar E. Iglesias > --- > +#define TYPE_XLNX_ZYNQMP_CRF "xlnx.zynqmp_crf" > + > +#define XILINX_CRF(obj) \ > + OBJECT_CHECK(XlnxZynqMPCRF, (obj), TYPE_XLNX_ZYNQMP_CRF) We prefer the OBJECT_DECLARE_SIMPLE_TYPE rather than directly defining a cast macro these days. (It also provides a typedef for you, among other things.) Apart from that, and dropping minimum_version_id_old, Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH V7 00/29] Live Update
Please? - Steve On 1/7/2022 1:45 PM, Steven Sistare wrote: > Hi Dave, > It has been a long time since we chatted about this series. The vfio > patches have been updated with feedback from Alex and are close to being > final (I think). Could you take another look at the patches that you care > about? To refresh your memory, you last reviewed V3 of the series, and I > made significant changes to address your comments. The cover letter lists > the changes in V4, V5, V6, and V7. > > Best wishes for the new year, > - Steve > > On 12/22/2021 2:05 PM, Steve Sistare wrote: >> Provide the cpr-save, cpr-exec, and cpr-load commands for live update. >> These save and restore VM state, with minimal guest pause time, so that >> qemu may be updated to a new version in between. >> >> cpr-save stops the VM and saves vmstate to an ordinary file. It supports >> any type of guest image and block device, but the caller must not modify >> guest block devices between cpr-save and cpr-load. It supports two modes: >> reboot and restart. >> >> In reboot mode, the caller invokes cpr-save and then terminates qemu. >> The caller may then update the host kernel and system software and reboot. >> The caller resumes the guest by running qemu with the same arguments as the >> original process and invoking cpr-load. To use this mode, guest ram must be >> mapped to a persistent shared memory file such as /dev/dax0.0, or /dev/shm >> PKRAM as proposed in >> https://lore.kernel.org/lkml/1617140178-8773-1-git-send-email-anthony.yzn...@oracle.com. >> >> The reboot mode supports vfio devices if the caller first suspends the >> guest, such as by issuing guest-suspend-ram to the qemu guest agent. The >> guest drivers' suspend methods flush outstanding requests and re-initialize >> the devices, and thus there is no device state to save and restore. >> >> Restart mode preserves the guest VM across a restart of the qemu process. >> After cpr-save, the caller passes qemu command-line arguments to cpr-exec, >> which directly exec's the new qemu binary. The arguments must include -S >> so new qemu starts in a paused state and waits for the cpr-load command. >> The restart mode supports vfio devices by preserving the vfio container, >> group, device, and event descriptors across the qemu re-exec, and by >> updating DMA mapping virtual addresses using VFIO_DMA_UNMAP_FLAG_VADDR and >> VFIO_DMA_MAP_FLAG_VADDR as defined in >> https://lore.kernel.org/kvm/1611939252-7240-1-git-send-email-steven.sist...@oracle.com/ >> and integrated in Linux kernel 5.12. >> >> To use the restart mode, qemu must be started with the memfd-alloc option, >> which allocates guest ram using memfd_create. The memfd's are saved to >> the environment and kept open across exec, after which they are found from >> the environment and re-mmap'd. Hence guest ram is preserved in place, >> albeit with new virtual addresses in the qemu process. >> >> The caller resumes the guest by invoking cpr-load, which loads state from >> the file. If the VM was running at cpr-save time, then VM execution resumes. >> If the VM was suspended at cpr-save time (reboot mode), then the caller must >> issue a system_wakeup command to resume. >> >> The first patches add reboot mode: >> - memory: qemu_check_ram_volatile >> - migration: fix populate_vfio_info >> - migration: qemu file wrappers >> - migration: simplify savevm >> - vl: start on wakeup request >> - cpr: reboot mode >> - cpr: reboot HMP interfaces >> >> The next patches add restart mode: >> - memory: flat section iterator >> - oslib: qemu_clear_cloexec >> - machine: memfd-alloc option >> - qapi: list utility functions >> - vl: helper to request re-exec >> - cpr: preserve extra state >> - cpr: restart mode >> - cpr: restart HMP interfaces >> - hostmem-memfd: cpr for memory-backend-memfd >> >> The next patches add vfio support for restart mode: >> - pci: export functions for cpr >> - vfio-pci: refactor for cpr >> - vfio-pci: cpr part 1 (fd and dma) >> - vfio-pci: cpr part 2 (msi) >> - vfio-pci: cpr part 3 (intx) >> - vfio-pci: recover from unmap-all-vaddr failure >> >> The next patches preserve various descriptor-based backend devices across >> cprexec: >> - loader: suppress rom_reset during cpr >> - vhost: reset vhost devices for cpr >> - chardev: cpr framework >> - chardev: cpr for simple devices >> - chardev: cpr for pty >> - chardev: cpr for sockets >> - cpr: only-cpr-capable option >> >> Here is an example of updating qemu from v4.2.0 to v4.2.1 using >> restart mode. The software update is performed while the guest is >> running to minimize downtime. >> >> window 1| window 2 >> | >> # qemu-system-x86_64 ...| >> QEMU 4.2.0 monitor - type 'help' ...| >> (qemu) info status | >> VM status: running
Re: [PATCH] MAINTAINERS: Add Akihiko Odaki to macOS-relateds
On Sun, 13 Feb 2022 at 02:12, Akihiko Odaki wrote:
>
> Signed-off-by: Akihiko Odaki
> ---
> MAINTAINERS | 2 ++
> 1 file changed, 2 insertions(+)
Thanks for volunteering to review cocoa-related patches!
Would you mind having a look at these couple of patches
from early January?
https://patchew.org/QEMU/20220102174153.70043-1-carwynel...@gmail.com/
("ui/cocoa: Add option to disable left command and hide cursor on click")
https://patchew.org/QEMU/20220103114515.24020-1-carwynel...@gmail.com/
("Show/hide the menu bar in fullscreen on mouse")
They need review both from the pov of "is this a sensible UI change
for the cococa frontend" and also the usual code-level review.
They've been on my "I should look at this" list for weeks now
but I don't really feel very confident to review on either front...
I'll take this patch via target-arm.next, since I'm making a pullreq
anyway.
thanks
-- PMM
Re: [PATCH] hw/timer: fix a9gtimer vmstate
On Mon, 14 Feb 2022 at 07:34, Pavel Dovgalyuk wrote: > > ping > > On 07.02.2022 11:44, Pavel Dovgalyuk wrote: > > A9 gtimer includes global control field and number of per-cpu fields. > > But only per-cpu ones are migrated. This patch adds a subsection for > > global control field migration. > > > > Signed-off-by: Pavel Dovgalyuk Thanks for the ping, this one fell through the net. Applied to target-arm.next, thanks. -- PMM
Re: [PATCH] build: fix build failure with gcc 11.2 by disabling -fcf-protection
On Tue, 8 Feb 2022 at 21:29, Vineet Gupta wrote: > > When doing RV qemu builds with host gcc 11.2, ran into following build failure > > | cc -MMD -MP -MT linuxboot_dma.o -MF ./linuxboot_dma.d -O2 -g -march=i486 > -Wall \ > | -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings > -Wmissing-prototypes \ > | -Wold-style-declaration -Wold-style-definition -Wtype-limits > -Wformat-security \ > | -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body > -Wnested-externs \ > | -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 > -Wno-missing-include-dirs \ > | -Wno-shift-negative-value -Wno-psabi -fno-pie -ffreestanding > -IQEMU/include \ > | -fno-stack-protector -m16 -Wa,-32 \ > | -c QEMU/pc-bios/optionrom/linuxboot_dma.c -o linuxboot_dma.o > |cc1: error: ‘-fcf-protection’ is not compatible with this target > > Signed-off-by: Vineet Gupta > --- > This might be a crude fix to the problem I think this patch https://patchew.org/QEMU/20220103090112.312202-1-bj...@kernel.org/ fixes the same problem... thanks -- PMM
Re: meson incremental build doesn't handle config file going away
On Fri, 18 Feb 2022 at 12:08, Thomas Huth wrote:
>
> On 18/02/2022 12.59, Peter Maydell wrote:
> > I've noticed that the meson incremental build doesn't seem to
> > cleanly handle a config file going away, as ppc64abi32-linux-user
> > has recently. The build fails with:
> >
> > ../../meson.build:1941:2: ERROR: Failed to load
> > /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/configs/targets/ppc64abi32-linux-user.mak:
> > [Errno 2] No such file or directory:
> > '/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/configs/targets/ppc64abi32-linux-user.mak'
> >
> > where line 1941 is
> >config_target += keyval.load('configs/targets' / target + '.mak')
> >
> > Rerunning make doesn't help; you have to manually re-run
> > configure.
> >
> > Something here should figure out that "config file deleted" means
> > it needs to rerun configure, I think.
>
> That's weird, since the patch that removed that target actually change the
> configure script, too, so it should have re-run configure afterwards... or
> is that broken, too?
I'm not sure exactly what's going on -- I didn't see this when I
originally applied the pullreq, but I have seen it a couple of times
later. I guess that some amount of switching between branches and
incrementally-building of older build trees might be involved.
-- PMM
