[RFC QEMU PATCH v3 1/1] xen: Use gsi instead of irq for mapping pirq

2023-12-10 Thread Jiqian Chen
In PVH dom0, it uses the linux local interrupt mechanism,
when it allocs irq for a gsi, it is dynamic, and follow
the principle of applying first, distributing first. And
the irq number is alloced from small to large, but the
applying gsi number is not, may gsi 38 comes before gsi
28, that causes the irq number is not equal with the gsi
number. And when passthrough a device, qemu wants to use
gsi to map pirq, xen_pt_realize->xc_physdev_map_pirq, but
the gsi number is got from file
/sys/bus/pci/devices//irq in current code, so it
will fail when mapping.

Use real gsi number read from gsi sysfs.

Co-developed-by: Huang Rui 
Signed-off-by: Jiqian Chen 
---
 hw/xen/xen-host-pci-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 8c6e9a1716..e270ac2631 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -364,7 +364,7 @@ void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t 
domain,
 }
 d->device_id = v;
 
-xen_host_pci_get_dec_value(d, "irq", &v, errp);
+xen_host_pci_get_dec_value(d, "gsi", &v, errp);
 if (*errp) {
 goto error;
 }
-- 
2.34.1




[RFC QEMU PATCH v3 0/1] Support device passthrough when dom0 is PVH on Xen

2023-12-10 Thread Jiqian Chen
Hi All,
v2->v3 changes:
* du to changes in the implementation of the second patch on kernel side(that 
adds a new sysfs for gsi instead of a new syscall), so read gsi number from the 
sysfs of gsi.


v3 patch on kernel side:
https://lore.kernel.org/lkml/20231210161519.1550860-1-jiqian.c...@amd.com/T/#t
v3 patch on Xen side:
https://lore.kernel.org/xen-devel/20231210164009.1551147-1-jiqian.c...@amd.com/T/#t


Below is the description of v2 cover letter:
This patch is the v2 of the implementation of passthrough when dom0 is PVH on 
Xen.
Issues we encountered:
1. failed to map pirq for gsi
Problem: qemu will call xc_physdev_map_pirq() to map a passthrough device’s gsi 
to pirq in function xen_pt_realize(). But failed.

Reason: According to the implement of xc_physdev_map_pirq(), it needs gsi 
instead of irq, but qemu pass irq to it and treat irq as gsi, it is got from 
file /sys/bus/pci/devices/:xx:xx.x/irq in function 
xen_host_pci_device_get(). But actually the gsi number is not equal with irq. 
On PVH dom0, when it allocates irq for a gsi in function 
acpi_register_gsi_ioapic(), allocation is dynamic, and follow the principle of 
applying first, distributing first. And if you debug the kernel codes(see 
function __irq_alloc_descs), you will find the irq number is allocated from 
small to large by order, but the applying gsi number is not, gsi 38 may come 
before gsi 28, that causes gsi 38 get a smaller irq number than gsi 28, and 
then gsi != irq.

Solution: we can record the relation between gsi and irq, then when 
userspace(qemu) want to use gsi, we can do a translation. The third patch of 
kernel(xen/privcmd: Add new syscall to get gsi from irq) records all the 
relations in acpi_register_gsi_xen_pvh() when dom0 initialize pci devices, and 
provide a syscall for userspace to get the gsi from irq. The third patch of 
xen(tools: Add new function to get gsi from irq) add a new function 
xc_physdev_gsi_from_irq() to call the new syscall added on kernel side.
And then userspace can use that function to get gsi. Then xc_physdev_map_pirq() 
will success.

This v2 on qemu side is the same as the v1 ( qemu 
https://lore.kernel.org/xen-devel/20230312092244.451465-19-ray.hu...@amd.com/), 
just call
xc_physdev_gsi_from_irq() to get gsi from irq.

v2 on kernel side:
https://lore.kernel.org/lkml/20231124103123.3263471-1-jiqian.c...@amd.com/T/#t

v2 on Xen side:
https://lore.kernel.org/xen-devel/20231124104136.3263722-1-jiqian.c...@amd.com/T/#t


Jiqian Chen (1):
  xen: Use gsi instead of irq for mapping pirq

 hw/xen/xen-host-pci-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.34.1




[PATCH qemu 0/3] [PATCH qemu 0/3] hw/arm: Add device STM32L4x5 SYSCFG

2023-12-10 Thread ~inesvarhol
This patch adds a new SYSCFG peripheral used by the STM32L4x5 SoC.
The implementation is inspired from the STM32F4xx SYSCFG, the device
registers however aren't the same.

The patch is split up in 3 commits :
- implementing the SYSCFG device
- adding tests (that fail in this commit)
- connecting the SYSCFG device to the SoC (the tests pass in this
commit)

Based-on: <170198462199.32162.28449757725342730...@git.sr.ht>
([PATCH qemu 0/3] hw/arm: Add device STM32L4x5 EXTI)

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 

Inès Varhol (3):
  hw/misc: Implement STM32L4xx SYSCFG
  tests/qtest: Add STM32L4xx SYSCFG QTest testcase
  hw/arm: Connect STM32L4xx SYSCFG to STM32L4x5 SoC

 hw/arm/Kconfig  |   1 +
 hw/arm/stm32l4x5_soc.c  |  24 +-
 hw/misc/Kconfig |   3 +
 hw/misc/meson.build |   1 +
 hw/misc/stm32l4xx_syscfg.c  | 277 +++
 hw/misc/trace-events|   6 +
 include/hw/arm/stm32l4x5_soc.h  |   2 +
 include/hw/misc/stm32l4xx_syscfg.h  |  62 +
 tests/qtest/meson.build |   3 +-
 tests/qtest/stm32l4xx_syscfg-test.c | 408 
 10 files changed, 778 insertions(+), 9 deletions(-)
 create mode 100644 hw/misc/stm32l4xx_syscfg.c
 create mode 100644 include/hw/misc/stm32l4xx_syscfg.h
 create mode 100644 tests/qtest/stm32l4xx_syscfg-test.c

-- 
2.38.5



[PATCH qemu 3/3] hw/arm: Connect STM32L4xx SYSCFG to STM32L4x5 SoC

2023-12-10 Thread ~inesvarhol
From: Inès Varhol 

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
---
 hw/arm/Kconfig |  1 +
 hw/arm/stm32l4x5_soc.c | 24 
 include/hw/arm/stm32l4x5_soc.h |  2 ++
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 9c9d5bb541..ea77977d4b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -458,6 +458,7 @@ config STM32L4X5_SOC
 bool
 select ARM_V7M
 select OR_IRQ
+select STM32L4XX_SYSCFG
 select STM32L4X5_EXTI
 
 config XLNX_ZYNQMP_ARM
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index cf786eac1d..9b45a9e606 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -46,6 +46,7 @@
 #define SRAM2_SIZE (32 * KiB)
 
 #define EXTI_ADDR 0x40010400
+#define SYSCFG_ADDR 0x4001
 
 #define NUM_EXTI_IRQ 40
 /* Match exti line connections with their CPU IRQ number */
@@ -90,6 +91,8 @@ static void stm32l4x5_soc_initfn(Object *obj)
 
 object_initialize_child(obj, "exti", &s->exti, TYPE_STM32L4X5_EXTI);
 
+object_initialize_child(obj, "syscfg", &s->syscfg, TYPE_STM32L4XX_SYSCFG);
+
 s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
 s->refclk = qdev_init_clock_in(DEVICE(s), "refclk", NULL, NULL, 0);
 }
@@ -167,6 +170,15 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
Error **errp)
 return;
 }
 
+/* System configuration controller */
+dev = DEVICE(&s->syscfg);
+if (!sysbus_realize(SYS_BUS_DEVICE(&s->syscfg), errp)) {
+return;
+}
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, SYSCFG_ADDR);
+
+/* EXTI device */
 dev = DEVICE(&s->exti);
 if (!sysbus_realize(SYS_BUS_DEVICE(&s->exti), errp)) {
 return;
@@ -178,13 +190,10 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
Error **errp)
 sysbus_connect_irq(busdev, i, qdev_get_gpio_in(armv7m, exti_irq[i]));
 }
 
-/*
- * Uncomment when Syscfg is implemented
- * for (i = 0; i < 16; i++) {
- * qdev_connect_gpio_out(DEVICE(&s->syscfg), i,
- *   qdev_get_gpio_in(dev, i));
- * }
- */
+for (i = 0; i < 16; i++) {
+qdev_connect_gpio_out(DEVICE(&s->syscfg), i,
+  qdev_get_gpio_in(dev, i));
+}
 
 /* APB1 BUS */
 create_unimplemented_device("TIM2",  0x4000, 0x400);
@@ -223,7 +232,6 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
Error **errp)
 /* RESERVED:0x40009800, 0x6800 */
 
 /* APB2 BUS */
-create_unimplemented_device("SYSCFG",0x4001, 0x30);
 create_unimplemented_device("VREFBUF",   0x40010030, 0x1D0);
 create_unimplemented_device("COMP",  0x40010200, 0x200);
 /* RESERVED:0x40010800, 0x1400 */
diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h
index ac47158596..5ff757f15b 100644
--- a/include/hw/arm/stm32l4x5_soc.h
+++ b/include/hw/arm/stm32l4x5_soc.h
@@ -37,6 +37,7 @@
 #include "qemu/units.h"
 #include "hw/qdev-core.h"
 #include "hw/arm/armv7m.h"
+#include "hw/misc/stm32l4xx_syscfg.h"
 #include "hw/misc/stm32l4x5_exti.h"
 #include "qom/object.h"
 
@@ -52,6 +53,7 @@ struct Stm32l4x5SocState {
 ARMv7MState armv7m;
 
 Stm32l4x5ExtiState exti;
+STM32L4xxSyscfgState syscfg;
 
 MemoryRegion sram1;
 MemoryRegion sram2;
-- 
2.38.5



[PATCH qemu 2/3] tests/qtest: Add STM32L4xx SYSCFG QTest testcase

2023-12-10 Thread ~inesvarhol
From: Inès Varhol 

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
---
 tests/qtest/meson.build |   3 +-
 tests/qtest/stm32l4xx_syscfg-test.c | 408 
 2 files changed, 410 insertions(+), 1 deletion(-)
 create mode 100644 tests/qtest/stm32l4xx_syscfg-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index d5126f4d86..06a04ed17d 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -196,7 +196,8 @@ qtests_aspeed = \
'aspeed_gpio-test']
 
 qtests_stm32l4x5 = \
-  ['stm32l4x5_exti-test']
+  ['stm32l4x5_exti-test',
+   'stm32l4xx_syscfg-test']
 
 qtests_arm = \
   (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
diff --git a/tests/qtest/stm32l4xx_syscfg-test.c 
b/tests/qtest/stm32l4xx_syscfg-test.c
new file mode 100644
index 00..166b596ba6
--- /dev/null
+++ b/tests/qtest/stm32l4xx_syscfg-test.c
@@ -0,0 +1,408 @@
+/*
+ * QTest testcase for STM32L4xx_SYSCFG
+ *
+ * Copyright (c) 2023 Arnaud Minier 
+ * Copyright (c) 2023 Inès Varhol 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+
+#define SYSCFG_BASE_ADDR 0x4001
+#define SYSCFG_MEMRMP 0x00
+#define SYSCFG_CFGR1 0x04
+#define SYSCFG_EXTICR1 0x08
+#define SYSCFG_EXTICR2 0x0C
+#define SYSCFG_EXTICR3 0x10
+#define SYSCFG_EXTICR4 0x14
+#define SYSCFG_SCSR 0x18
+#define SYSCFG_CFGR2 0x1C
+#define SYSCFG_SWPR 0x20
+#define SYSCFG_SKR 0x24
+#define SYSCFG_SWPR2 0x28
+#define INVALID_ADDR 0x2C
+
+#define EXTI_BASE_ADDR 0x40010400
+#define EXTI_IMR1 0x00
+#define EXTI_RTSR1 0x08
+#define EXTI_FTSR1 0x0C
+
+static void syscfg_writel(unsigned int offset, uint32_t value)
+{
+writel(SYSCFG_BASE_ADDR + offset, value);
+}
+
+static uint32_t syscfg_readl(unsigned int offset)
+{
+return readl(SYSCFG_BASE_ADDR + offset);
+}
+
+static void exti_writel(unsigned int offset, uint32_t value)
+{
+writel(EXTI_BASE_ADDR + offset, value);
+}
+
+static void system_reset(void)
+{
+QDict *response;
+response = qtest_qmp(global_qtest, "{'execute': 'system_reset'}");
+g_assert(qdict_haskey(response, "return"));
+qobject_unref(response);
+}
+
+static void test_reset(void)
+{
+/*
+ * Test that registers are initialized at the correct values
+ */
+const uint32_t memrmp = syscfg_readl(SYSCFG_MEMRMP);
+g_assert_cmpuint(memrmp, ==, 0x);
+
+const uint32_t cfgr1 = syscfg_readl(SYSCFG_CFGR1);
+g_assert_cmpuint(cfgr1, ==, 0x7C01);
+
+const uint32_t exticr1 = syscfg_readl(SYSCFG_EXTICR1);
+g_assert_cmpuint(exticr1, ==, 0x);
+
+const uint32_t exticr2 = syscfg_readl(SYSCFG_EXTICR2);
+g_assert_cmpuint(exticr2, ==, 0x);
+
+const uint32_t exticr3 = syscfg_readl(SYSCFG_EXTICR3);
+g_assert_cmpuint(exticr3, ==, 0x);
+
+const uint32_t exticr4 = syscfg_readl(SYSCFG_EXTICR4);
+g_assert_cmpuint(exticr4, ==, 0x);
+
+const uint32_t scsr = syscfg_readl(SYSCFG_SCSR);
+g_assert_cmpuint(scsr, ==, 0x);
+
+const uint32_t cfgr2 = syscfg_readl(SYSCFG_CFGR2);
+g_assert_cmpuint(cfgr2, ==, 0x);
+
+const uint32_t swpr = syscfg_readl(SYSCFG_SWPR);
+g_assert_cmpuint(swpr, ==, 0x);
+
+const uint32_t skr = syscfg_readl(SYSCFG_SKR);
+g_assert_cmpuint(skr, ==, 0x);
+
+const uint32_t swpr2 = syscfg_readl(SYSCFG_SWPR2);
+g_assert_cmpuint(swpr2, ==, 0x);
+}
+
+static void test_reserved_bits(void)
+{
+/*
+ * Test that reserved bits stay at reset value
+ * (which is 0 for all of them) by writing '1'
+ * in all reserved bits (keeping reset value for
+ * other bits) and checking that the
+ * register is still at reset value
+ */
+syscfg_writel(SYSCFG_MEMRMP, 0xFEF8);
+const uint32_t memrmp = syscfg_readl(SYSCFG_MEMRMP);
+g_assert_cmpuint(memrmp, ==, 0x);
+
+syscfg_writel(SYSCFG_CFGR1, 0x7F00FEFF);
+const uint32_t cfgr1 = syscfg_readl(SYSCFG_CFGR1);
+g_assert_cmpuint(cfgr1, ==, 0x7C01);
+
+syscfg_writel(SYSCFG_EXTICR1, 0x);
+const uint32_t exticr1 = syscfg_readl(SYSCFG_EXTICR1);
+g_assert_cmpuint(exticr1, ==, 0x);
+
+syscfg_writel(SYSCFG_EXTICR2, 0x);
+const uint32_t exticr2 = syscfg_readl(SYSCFG_EXTICR2);
+g_assert_cmpuint(exticr2, ==, 0x);
+
+syscfg_writel(SYSCFG_EXTICR3, 0x);
+const uint32_t exticr3 = syscfg_readl(SYSCFG_EXTICR3);
+g_assert_cmpuint(exticr3, ==, 0x);
+
+syscfg_writel(SYSCFG_EXTICR4, 0x);
+const uint32_t exticr4 = syscfg_readl(SYSCFG_EXTICR4);
+g_assert_cmpuint(exticr4, ==, 0x);
+
+syscfg_writel(SYSCFG_SKR, 0xFF00);
+const uint32_t skr = syscfg_readl(SYSCFG_SKR);
+g_assert_cmpuint(skr, ==, 0x);
+}
+
+static void test_set_and_clear(void)
+{
+

[PATCH qemu 1/3] hw/misc: Implement STM32L4xx SYSCFG

2023-12-10 Thread ~inesvarhol
From: Inès Varhol 

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
---
 hw/misc/Kconfig|   3 +
 hw/misc/meson.build|   1 +
 hw/misc/stm32l4xx_syscfg.c | 277 +
 hw/misc/trace-events   |   6 +
 include/hw/misc/stm32l4xx_syscfg.h |  62 +++
 5 files changed, 349 insertions(+)
 create mode 100644 hw/misc/stm32l4xx_syscfg.c
 create mode 100644 include/hw/misc/stm32l4xx_syscfg.h

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 3efe3dc2cc..a20427a9be 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -90,6 +90,9 @@ config STM32F4XX_EXTI
 config STM32L4X5_EXTI
 bool
 
+config STM32L4XX_SYSCFG
+bool
+
 config MIPS_ITU
 bool
 
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 16db6e228d..120c59d9f4 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -111,6 +111,7 @@ system_ss.add(when: 'CONFIG_STM32F2XX_SYSCFG', if_true: 
files('stm32f2xx_syscfg.
 system_ss.add(when: 'CONFIG_STM32F4XX_SYSCFG', if_true: 
files('stm32f4xx_syscfg.c'))
 system_ss.add(when: 'CONFIG_STM32F4XX_EXTI', if_true: 
files('stm32f4xx_exti.c'))
 system_ss.add(when: 'CONFIG_STM32L4X5_EXTI', if_true: 
files('stm32l4x5_exti.c'))
+system_ss.add(when: 'CONFIG_STM32L4XX_SYSCFG', if_true: 
files('stm32l4xx_syscfg.c'))
 system_ss.add(when: 'CONFIG_MPS2_FPGAIO', if_true: files('mps2-fpgaio.c'))
 system_ss.add(when: 'CONFIG_MPS2_SCC', if_true: files('mps2-scc.c'))
 
diff --git a/hw/misc/stm32l4xx_syscfg.c b/hw/misc/stm32l4xx_syscfg.c
new file mode 100644
index 00..a8ef370117
--- /dev/null
+++ b/hw/misc/stm32l4xx_syscfg.c
@@ -0,0 +1,277 @@
+/*
+ * STM32L4xx SYSCFG (System Configuration Controller)
+ *
+ * Copyright (c) 2014 Alistair Francis 
+ * Copyright (c) 2023 Arnaud Minier 
+ * Copyright (c) 2023 Inès Varhol 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * 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.
+ *
+ * Based on the stm32f4xx_syscfg by Alistair Francis.
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ * 
https://www.st.com/en/microcontrollers-microprocessors/stm32l4x5/documentation.html
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "hw/misc/stm32l4xx_syscfg.h"
+
+#define NUM_GPIOS 7
+
+#define SYSCFG_MEMRMP 0x00
+#define SYSCFG_CFGR1 0x04
+#define SYSCFG_EXTICR1 0x08
+#define SYSCFG_EXTICR2 0x0C
+#define SYSCFG_EXTICR3 0x10
+#define SYSCFG_EXTICR4 0x14
+#define SYSCFG_SCSR 0x18
+#define SYSCFG_CFGR2 0x1C
+#define SYSCFG_SWPR 0x20
+#define SYSCFG_SKR 0x24
+#define SYSCFG_SWPR2 0x28
+
+/* __0001_0111 */
+#define ACTIVABLE_BITS_MEMRP 0x0107
+
+/* 1100__0001_ */
+#define ACTIVABLE_BITS_CFGR1 0xFCFF0100
+/* ___0001 */
+#define FIREWALL_DISABLE_CFGR1 0x0001
+
+/* ___0001 */
+#define ACTIVABLE_BITS_EXTICR 0x
+
+/* ___0011 */
+/* #define ACTIVABLE_BITS_SCSR 0x0003 */
+
+/* ___ */
+#define ECC_LOCK_CFGR2 0x000F
+/* __0001_ */
+#define SRAM2_PARITY_ERROR_FLAG_CFGR2 0x0100
+
+/* ___ */
+#define ACTIVABLE_BITS_SKR 0x00FF
+
+static void stm32l4xx_syscfg_hold_reset(Object *obj)
+{
+STM32L4xxSyscfgState *s = STM32L4XX_SYSCFG(obj);
+
+s->memrmp = 0x;
+s->cfgr1 = 0x7C01;
+s->exticr[0] = 0x;
+s->exticr[1] = 0x;
+s->exticr[2] = 0x;
+s->exticr[3] = 0x;
+s->scsr = 0x;
+s->cfgr2 = 0x;
+s->swpr = 0x;
+s->skr = 0x;
+s->swpr2 = 0x;
+}
+
+static void stm32l4xx_syscfg_set_irq(void *opaque, int irq, int level)
+{
+STM32L4xxSyscfgState

[PATCH] target/i386: Give IRQs a chance when resetting HF_INHIBIT_IRQ_MASK

2023-12-10 Thread Ruihan Li
When emulated with QEMU, interrupts will never come in the following
loop. However, if the NOP instruction is uncommented, interrupts will
fire as normal.

loop:
cli
call do_sti
jmp loop

do_sti:
sti
# nop
ret

This behavior is different from that of a real processor. For example,
if KVM is enabled, interrupts will always fire regardless of whether the
NOP instruction is commented or not. Also, the Intel Software Developer
Manual states that after the STI instruction is executed, the interrupt
inhibit should end as soon as the next instruction (e.g., the RET
instruction if the NOP instruction is commented) is executed.

This problem is caused because the previous code may choose not to end
the TB even if the HF_INHIBIT_IRQ_MASK has just been reset (e.g., in the
case where the RET instruction is immediately followed by the STI
instruction), so that IRQs may not have a change to trigger. This commit
fixes the problem by always terminating the current TB to give IRQs a
chance to trigger when HF_INHIBIT_IRQ_MASK is reset.

Signed-off-by: Ruihan Li 
---
The same problem was discovered two years ago, see [StackOverflow][so].

 [so]: 
https://stackoverflow.com/questions/68135305/executing-ret-after-sti-doesnt-start-interrupts

 target/i386/tcg/translate.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 587d886..6b7deb5 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2767,13 +2767,19 @@ static void gen_bnd_jmp(DisasContext *s)
 static void
 do_gen_eob_worker(DisasContext *s, bool inhibit, bool recheck_tf, bool jr)
 {
+bool inhibit_reset;
+
 gen_update_cc_op(s);
 
 /* If several instructions disable interrupts, only the first does it.  */
 if (inhibit && !(s->flags & HF_INHIBIT_IRQ_MASK)) {
 gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
-} else {
+inhibit_reset = false;
+} else if (!inhibit && (s->flags & HF_INHIBIT_IRQ_MASK)) {
 gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
+inhibit_reset = true;
+} else {
+inhibit_reset = false;
 }
 
 if (s->base.tb->flags & HF_RF_MASK) {
@@ -2784,7 +2790,9 @@ do_gen_eob_worker(DisasContext *s, bool inhibit, bool 
recheck_tf, bool jr)
 tcg_gen_exit_tb(NULL, 0);
 } else if (s->flags & HF_TF_MASK) {
 gen_helper_single_step(tcg_env);
-} else if (jr) {
+} else if (jr &&
+   /* give irqs a chance to happen */
+   !inhibit_reset) {
 tcg_gen_lookup_and_goto_ptr();
 } else {
 tcg_gen_exit_tb(NULL, 0);
-- 
2.43.0




[PATCH v2 6/9] Hexagon (target/hexagon) Make generators object oriented - gen_op_regs

2023-12-10 Thread Taylor Simpson
Reviewed-by: Brian Cain 
Signed-off-by: Taylor Simpson 
---
 target/hexagon/gen_op_regs.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/hexagon/gen_op_regs.py b/target/hexagon/gen_op_regs.py
index a8a7712129..7b7b33895a 100755
--- a/target/hexagon/gen_op_regs.py
+++ b/target/hexagon/gen_op_regs.py
@@ -70,6 +70,7 @@ def strip_reg_prefix(x):
 def main():
 hex_common.read_semantics_file(sys.argv[1])
 hex_common.read_attribs_file(sys.argv[2])
+hex_common.init_registers()
 tagregs = hex_common.get_tagregs(full=True)
 tagimms = hex_common.get_tagimms()
 
@@ -80,11 +81,12 @@ def main():
 wregs = []
 regids = ""
 for regtype, regid, _, numregs in regs:
-if hex_common.is_read(regid):
+reg = hex_common.get_register(tag, regtype, regid)
+if reg.is_read():
 if regid[0] not in regids:
 regids += regid[0]
 rregs.append(regtype + regid + numregs)
-if hex_common.is_written(regid):
+if reg.is_written():
 wregs.append(regtype + regid + numregs)
 if regid[0] not in regids:
 regids += regid[0]
-- 
2.34.1




[PATCH v2 1/9] Hexagon (target/hexagon) Clean up handling of modifier registers

2023-12-10 Thread Taylor Simpson
Currently, the register number (MuN) for modifier registers is the
modifier register number rather than the index into hex_gpr.  This
patch changes MuN to the hex_gpr index, which is consistent with
the handling of control registers.

Note that HELPER(fcircadd) needs the CS register corresponding to the
modifier register specified in the instruction.  We create a TCGv
variable "CS" to hold the value to pass to the helper.

Reviewed-by: Brian Cain 
Signed-off-by: Taylor Simpson 
---
 target/hexagon/gen_tcg.h|  9 -
 target/hexagon/macros.h |  3 +--
 target/hexagon/idef-parser/parser-helpers.c |  8 +++-
 target/hexagon/gen_tcg_funcs.py | 13 +
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index d992059fce..1c4391b415 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -68,15 +68,14 @@
 do { \
 TCGv tcgv_siV = tcg_constant_tl(siV); \
 tcg_gen_mov_tl(EA, RxV); \
-gen_helper_fcircadd(RxV, RxV, tcgv_siV, MuV, \
-hex_gpr[HEX_REG_CS0 + MuN]); \
+gen_helper_fcircadd(RxV, RxV, tcgv_siV, MuV, CS); \
 } while (0)
 #define GET_EA_pcr(SHIFT) \
 do { \
 TCGv ireg = tcg_temp_new(); \
 tcg_gen_mov_tl(EA, RxV); \
 gen_read_ireg(ireg, MuV, (SHIFT)); \
-gen_helper_fcircadd(RxV, RxV, ireg, MuV, hex_gpr[HEX_REG_CS0 + MuN]); \
+gen_helper_fcircadd(RxV, RxV, ireg, MuV, CS); \
 } while (0)
 
 /* Instructions with multiple definitions */
@@ -113,7 +112,7 @@
 TCGv ireg = tcg_temp_new(); \
 tcg_gen_mov_tl(EA, RxV); \
 gen_read_ireg(ireg, MuV, SHIFT); \
-gen_helper_fcircadd(RxV, RxV, ireg, MuV, hex_gpr[HEX_REG_CS0 + MuN]); \
+gen_helper_fcircadd(RxV, RxV, ireg, MuV, CS); \
 LOAD; \
 } while (0)
 
@@ -427,7 +426,7 @@
 TCGv BYTE G_GNUC_UNUSED = tcg_temp_new(); \
 tcg_gen_mov_tl(EA, RxV); \
 gen_read_ireg(ireg, MuV, SHIFT); \
-gen_helper_fcircadd(RxV, RxV, ireg, MuV, hex_gpr[HEX_REG_CS0 + MuN]); \
+gen_helper_fcircadd(RxV, RxV, ireg, MuV, CS); \
 STORE; \
 } while (0)
 
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 9a51b5709b..939f22e76b 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -462,8 +462,7 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int 
shift)
 #define fPM_CIRI(REG, IMM, MVAL) \
 do { \
 TCGv tcgv_siV = tcg_constant_tl(siV); \
-gen_helper_fcircadd(REG, REG, tcgv_siV, MuV, \
-hex_gpr[HEX_REG_CS0 + MuN]); \
+gen_helper_fcircadd(REG, REG, tcgv_siV, MuV, CS); \
 } while (0)
 #else
 #define fEA_IMM(IMM)do { EA = (IMM); } while (0)
diff --git a/target/hexagon/idef-parser/parser-helpers.c 
b/target/hexagon/idef-parser/parser-helpers.c
index 4af020933a..95f2b43076 100644
--- a/target/hexagon/idef-parser/parser-helpers.c
+++ b/target/hexagon/idef-parser/parser-helpers.c
@@ -1541,10 +1541,8 @@ void gen_circ_op(Context *c,
  HexValue *increment,
  HexValue *modifier)
 {
-HexValue cs = gen_tmp(c, locp, 32, UNSIGNED);
 HexValue increment_m = *increment;
 increment_m = rvalue_materialize(c, locp, &increment_m);
-OUT(c, locp, "gen_read_reg(", &cs, ", HEX_REG_CS0 + MuN);\n");
 OUT(c,
 locp,
 "gen_helper_fcircadd(",
@@ -1555,7 +1553,7 @@ void gen_circ_op(Context *c,
 &increment_m,
 ", ",
 modifier);
-OUT(c, locp, ", ", &cs, ");\n");
+OUT(c, locp, ", CS);\n");
 }
 
 HexValue gen_locnt_op(Context *c, YYLTYPE *locp, HexValue *src)
@@ -2080,9 +2078,9 @@ void emit_arg(Context *c, YYLTYPE *locp, HexValue *arg)
 char reg_id[5];
 reg_compose(c, locp, &(arg->reg), reg_id);
 EMIT_SIG(c, ", %s %s", type, reg_id);
-/* MuV register requires also MuN to provide its index */
+/* MuV register requires also CS for circular addressing*/
 if (arg->reg.type == MODIFIER) {
-EMIT_SIG(c, ", int MuN");
+EMIT_SIG(c, ", TCGv CS");
 }
 }
 break;
diff --git a/target/hexagon/gen_tcg_funcs.py b/target/hexagon/gen_tcg_funcs.py
index f5246cee6d..02d93bc5ce 100755
--- a/target/hexagon/gen_tcg_funcs.py
+++ b/target/hexagon/gen_tcg_funcs.py
@@ -99,10 +99,15 @@ def genptr_decl(f, tag, regtype, regid, regno):
 hex_common.bad_register(regtype, regid)
 elif regtype == "M":
 if regid == "u":
-f.write(f"const int {regtype}{regid}N = " 
f"insn->regno[{regno}];\n")
 f.write(
-f"TCGv {regtype}{regid}V = hex_gpr[{regtype}{regid}N + "
-"HEX_REG_M0];\n"
+f"const int {regN} = insn->regno[{regno}] + HEX_REG_M0;\n"
+)
+f.write(
+   

[PATCH v2 3/9] Hexagon (target/hexagon) Make generators object oriented - gen_helper_protos

2023-12-10 Thread Taylor Simpson
Signed-off-by: Taylor Simpson 
---
 target/hexagon/gen_helper_protos.py | 149 ++--
 target/hexagon/hex_common.py|   7 --
 2 files changed, 8 insertions(+), 148 deletions(-)

diff --git a/target/hexagon/gen_helper_protos.py 
b/target/hexagon/gen_helper_protos.py
index 131043795a..c82b0f54e4 100755
--- a/target/hexagon/gen_helper_protos.py
+++ b/target/hexagon/gen_helper_protos.py
@@ -22,39 +22,6 @@
 import string
 import hex_common
 
-##
-## Helpers for gen_helper_prototype
-##
-def_helper_types = {
-"N": "s32",
-"O": "s32",
-"P": "s32",
-"M": "s32",
-"C": "s32",
-"R": "s32",
-"V": "ptr",
-"Q": "ptr",
-}
-
-def_helper_types_pair = {
-"R": "s64",
-"C": "s64",
-"S": "s64",
-"G": "s64",
-"V": "ptr",
-"Q": "ptr",
-}
-
-
-def gen_def_helper_opn(f, tag, regtype, regid, i):
-if hex_common.is_pair(regid):
-f.write(f", {def_helper_types_pair[regtype]}")
-elif hex_common.is_single(regid):
-f.write(f", {def_helper_types[regtype]}")
-else:
-hex_common.bad_register(regtype, regid)
-
-
 ##
 ## Generate the DEF_HELPER prototype for an instruction
 ## For A2_add: Rd32=add(Rs32,Rt32)
@@ -65,116 +32,15 @@ def gen_helper_prototype(f, tag, tagregs, tagimms):
 regs = tagregs[tag]
 imms = tagimms[tag]
 
-numresults = 0
-numscalarresults = 0
-numscalarreadwrite = 0
-for regtype, regid in regs:
-if hex_common.is_written(regid):
-numresults += 1
-if hex_common.is_scalar_reg(regtype):
-numscalarresults += 1
-if hex_common.is_readwrite(regid):
-if hex_common.is_scalar_reg(regtype):
-numscalarreadwrite += 1
-
-if numscalarresults > 1:
-## The helper is bogus when there is more than one result
-f.write(f"DEF_HELPER_1({tag}, void, env)\n")
-else:
-## Figure out how many arguments the helper will take
-if numscalarresults == 0:
-def_helper_size = len(regs) + len(imms) + numscalarreadwrite + 1
-if hex_common.need_pkt_has_multi_cof(tag):
-def_helper_size += 1
-if hex_common.need_pkt_need_commit(tag):
-def_helper_size += 1
-if hex_common.need_part1(tag):
-def_helper_size += 1
-if hex_common.need_slot(tag):
-def_helper_size += 1
-if hex_common.need_PC(tag):
-def_helper_size += 1
-if hex_common.helper_needs_next_PC(tag):
-def_helper_size += 1
-if hex_common.need_condexec_reg(tag, regs):
-def_helper_size += 1
-f.write(f"DEF_HELPER_{def_helper_size}({tag}")
-## The return type is void
-f.write(", void")
-else:
-def_helper_size = len(regs) + len(imms) + numscalarreadwrite
-if hex_common.need_pkt_has_multi_cof(tag):
-def_helper_size += 1
-if hex_common.need_pkt_need_commit(tag):
-def_helper_size += 1
-if hex_common.need_part1(tag):
-def_helper_size += 1
-if hex_common.need_slot(tag):
-def_helper_size += 1
-if hex_common.need_PC(tag):
-def_helper_size += 1
-if hex_common.need_condexec_reg(tag, regs):
-def_helper_size += 1
-if hex_common.helper_needs_next_PC(tag):
-def_helper_size += 1
-f.write(f"DEF_HELPER_{def_helper_size}({tag}")
-
-## Generate the qemu DEF_HELPER type for each result
-## Iterate over this list twice
-## - Emit the scalar result
-## - Emit the vector result
-i = 0
-for regtype, regid in regs:
-if hex_common.is_written(regid):
-if not hex_common.is_hvx_reg(regtype):
-gen_def_helper_opn(f, tag, regtype, regid, i)
-i += 1
-
-## Put the env between the outputs and inputs
-f.write(", env")
-i += 1
-
-# Second pass
-for regtype, regid in regs:
-if hex_common.is_written(regid):
-if hex_common.is_hvx_reg(regtype):
-gen_def_helper_opn(f, tag, regtype, regid, i)
-i += 1
-
-## For conditional instructions, we pass in the destination register
-if "A_CONDEXEC" in hex_common.attribdict[tag]:
-for regtype, regid in regs:
-if hex_common.is_writeonly(regid) and not 
hex_common.is_hvx_reg(
-regtype
-):
-gen_def_helper_opn(f, tag, regtype, regid, i)
-i += 1
+declared = []
+ret_type = hex_common.helper_ret_type(tag, regs).proto_arg
+declared.append(ret_type)
 
-## Generate the qemu type for each input operand (regs and immediates)
-for regtype, regid in 

[PATCH v2 0/9] Hexagon (target/hexagon) Make generators object oriented

2023-12-10 Thread Taylor Simpson
See commit message in second patch

 Changes in v2 
Address feedback from Brian Cain 
- Consolidate logic to create helper arg lists


Taylor Simpson (9):
  Hexagon (target/hexagon) Clean up handling of modifier registers
  Hexagon (target/hexagon) Make generators object oriented -
gen_tcg_funcs
  Hexagon (target/hexagon) Make generators object oriented -
gen_helper_protos
  Hexagon (target/hexagon) Make generators object oriented -
gen_helper_funcs
  Hexagon (target/hexagon) Make generators object oriented -
gen_idef_parser_funcs
  Hexagon (target/hexagon) Make generators object oriented - gen_op_regs
  Hexagon (target/hexagon) Make generators object oriented -
gen_analyze_funcs
  Hexagon (target/hexagon) Remove unused WRITES_PRED_REG attribute
  Hexagon (target/hexagon) Remove dead functions from hex_common.py

 target/hexagon/gen_tcg.h|   9 +-
 target/hexagon/macros.h |   3 +-
 target/hexagon/attribs_def.h.inc|   1 -
 target/hexagon/idef-parser/parser-helpers.c |   8 +-
 target/hexagon/gen_analyze_funcs.py | 163 +---
 target/hexagon/gen_helper_funcs.py  | 368 ++--
 target/hexagon/gen_helper_protos.py | 149 +---
 target/hexagon/gen_idef_parser_funcs.py |  20 +-
 target/hexagon/gen_op_regs.py   |   6 +-
 target/hexagon/gen_tcg_funcs.py | 566 +---
 target/hexagon/hex_common.py| 921 ++--
 11 files changed, 964 insertions(+), 1250 deletions(-)

-- 
2.34.1




[PATCH v2 2/9] Hexagon (target/hexagon) Make generators object oriented - gen_tcg_funcs

2023-12-10 Thread Taylor Simpson
The generators are generally a bunch of Python if-then-else
statements based on the regtype and regid.  Encapsulate regtype/regid
into a class hierarchy.  Clients lookup the register and invoke
methods.

This has several advantages for making the code easier to read,
understand, and maintain
- The class name makes it more clear what the operand does
- All the methods for a given type of operand are together
- Don't need hex_common.bad_register
  If a regtype/regid is missing, the lookup in hex_common.get_register
  will fail
- We can remove the functions in hex_common that use regtype/regid
  (e.g., is_read)

This patch creates the class hierarchy in hex_common and converts
gen_tcg_funcs.py.  The other scripts will be converted in subsequent
patches in this series.

Signed-off-by: Taylor Simpson 
---
 target/hexagon/gen_tcg_funcs.py | 571 ++-
 target/hexagon/hex_common.py| 659 
 2 files changed, 683 insertions(+), 547 deletions(-)

diff --git a/target/hexagon/gen_tcg_funcs.py b/target/hexagon/gen_tcg_funcs.py
index 02d93bc5ce..3d8e3cb6a2 100755
--- a/target/hexagon/gen_tcg_funcs.py
+++ b/target/hexagon/gen_tcg_funcs.py
@@ -23,466 +23,13 @@
 import hex_common
 
 
-##
-## Helpers for gen_tcg_func
-##
-def gen_decl_ea_tcg(f, tag):
-f.write("TCGv EA G_GNUC_UNUSED = tcg_temp_new();\n")
-
-
-def genptr_decl_pair_writable(f, tag, regtype, regid, regno):
-regN = f"{regtype}{regid}N"
-if regtype == "R":
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-elif regtype == "C":
-f.write(f"const int {regN} = insn->regno[{regno}] + 
HEX_REG_SA0;\n")
-else:
-hex_common.bad_register(regtype, regid)
-f.write(f"TCGv_i64 {regtype}{regid}V = " f"get_result_gpr_pair(ctx, 
{regN});\n")
-
-
-def genptr_decl_writable(f, tag, regtype, regid, regno):
-regN = f"{regtype}{regid}N"
-if regtype == "R":
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"TCGv {regtype}{regid}V = get_result_gpr(ctx, {regN});\n")
-elif regtype == "C":
-f.write(f"const int {regN} = insn->regno[{regno}] + 
HEX_REG_SA0;\n")
-f.write(f"TCGv {regtype}{regid}V = get_result_gpr(ctx, {regN});\n")
-elif regtype == "P":
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"TCGv {regtype}{regid}V = tcg_temp_new();\n")
-else:
-hex_common.bad_register(regtype, regid)
-
-
-def genptr_decl(f, tag, regtype, regid, regno):
-regN = f"{regtype}{regid}N"
-if regtype == "R":
-if regid in {"ss", "tt"}:
-f.write(f"TCGv_i64 {regtype}{regid}V = tcg_temp_new_i64();\n")
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-elif regid in {"dd", "ee", "xx", "yy"}:
-genptr_decl_pair_writable(f, tag, regtype, regid, regno)
-elif regid in {"s", "t", "u", "v"}:
-f.write(
-f"TCGv {regtype}{regid}V = " 
f"hex_gpr[insn->regno[{regno}]];\n"
-)
-elif regid in {"d", "e", "x", "y"}:
-genptr_decl_writable(f, tag, regtype, regid, regno)
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "P":
-if regid in {"s", "t", "u", "v"}:
-f.write(
-f"TCGv {regtype}{regid}V = " 
f"hex_pred[insn->regno[{regno}]];\n"
-)
-elif regid in {"d", "e", "x"}:
-genptr_decl_writable(f, tag, regtype, regid, regno)
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "C":
-if regid == "ss":
-f.write(f"TCGv_i64 {regtype}{regid}V = " 
f"tcg_temp_new_i64();\n")
-f.write(f"const int {regN} = insn->regno[{regno}] + " 
"HEX_REG_SA0;\n")
-elif regid == "dd":
-genptr_decl_pair_writable(f, tag, regtype, regid, regno)
-elif regid == "s":
-f.write(f"TCGv {regtype}{regid}V = tcg_temp_new();\n")
-f.write(
-f"const int {regtype}{regid}N = insn->regno[{regno}] + "
-"HEX_REG_SA0;\n"
-)
-elif regid == "d":
-genptr_decl_writable(f, tag, regtype, regid, regno)
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "M":
-if regid == "u":
-f.write(
-f"const int {regN} = insn->regno[{regno}] + HEX_REG_M0;\n"
-)
-f.write(
-f"TCGv {regtype}{regid}V = hex_gpr[{regN}];\n"
-)
-f.write(
-f"TCGv CS G_GNUC_UNUSED = "
-f"hex_gpr[{regN} - HEX_REG_M0 + HEX_REG_CS0];\n"
-)
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "V":
-if regid in {"dd"}:
-f.write(f"const int {regtype}{regid}N = " 
f"insn->regno[{regno}

[PATCH v2 9/9] Hexagon (target/hexagon) Remove dead functions from hex_common.py

2023-12-10 Thread Taylor Simpson
These functions are no longer used after making the generators
object oriented.

Signed-off-by: Taylor Simpson 
---
 target/hexagon/hex_common.py | 51 
 1 file changed, 51 deletions(-)

diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index ca5e9630c1..195620c7ec 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -33,9 +33,6 @@
 overrides = {}  # tags with helper overrides
 idef_parser_enabled = {}  # tags enabled for idef-parser
 
-def bad_register(regtype, regid):
-raise Exception(f"Bad register parse: regtype '{regtype}' regid '{regid}'")
-
 # We should do this as a hash for performance,
 # but to keep order let's keep it as a list.
 def uniquify(seq):
@@ -200,46 +197,6 @@ def get_tagimms():
 return dict(zip(tags, list(map(compute_tag_immediates, tags
 
 
-def is_pair(regid):
-return len(regid) == 2
-
-
-def is_single(regid):
-return len(regid) == 1
-
-
-def is_written(regid):
-return regid[0] in "dexy"
-
-
-def is_writeonly(regid):
-return regid[0] in "de"
-
-
-def is_read(regid):
-return regid[0] in "stuvwxy"
-
-
-def is_readwrite(regid):
-return regid[0] in "xy"
-
-
-def is_scalar_reg(regtype):
-return regtype in "RPC"
-
-
-def is_hvx_reg(regtype):
-return regtype in "VQ"
-
-
-def is_old_val(regtype, regid, tag):
-return regtype + regid + "V" in semdict[tag]
-
-
-def is_new_val(regtype, regid, tag):
-return regtype + regid + "N" in semdict[tag]
-
-
 def need_slot(tag):
 if (
 "A_CVI_SCATTER" not in attribdict[tag]
@@ -280,14 +237,6 @@ def skip_qemu_helper(tag):
 return tag in overrides.keys()
 
 
-def is_tmp_result(tag):
-return "A_CVI_TMP" in attribdict[tag] or "A_CVI_TMP_DST" in attribdict[tag]
-
-
-def is_new_result(tag):
-return "A_CVI_NEW" in attribdict[tag]
-
-
 def is_idef_parser_enabled(tag):
 return tag in idef_parser_enabled
 
-- 
2.34.1




[PATCH v2 7/9] Hexagon (target/hexagon) Make generators object oriented - gen_analyze_funcs

2023-12-10 Thread Taylor Simpson
This patch conflicts with
https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg00729.html
If that series goes in first, we'll rework this patch and vice versa.

Signed-off-by: Taylor Simpson 
---
 target/hexagon/gen_analyze_funcs.py | 163 +---
 target/hexagon/hex_common.py| 151 ++
 2 files changed, 157 insertions(+), 157 deletions(-)

diff --git a/target/hexagon/gen_analyze_funcs.py 
b/target/hexagon/gen_analyze_funcs.py
index c3b521abef..a9af666cef 100755
--- a/target/hexagon/gen_analyze_funcs.py
+++ b/target/hexagon/gen_analyze_funcs.py
@@ -23,162 +23,6 @@
 import hex_common
 
 
-##
-## Helpers for gen_analyze_func
-##
-def is_predicated(tag):
-return "A_CONDEXEC" in hex_common.attribdict[tag]
-
-
-def analyze_opn_old(f, tag, regtype, regid, regno):
-regN = f"{regtype}{regid}N"
-predicated = "true" if is_predicated(tag) else "false"
-if regtype == "R":
-if regid in {"ss", "tt"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_reg_read_pair(ctx, {regN});\n")
-elif regid in {"dd", "ee", "xx", "yy"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_reg_write_pair(ctx, {regN}, 
{predicated});\n")
-elif regid in {"s", "t", "u", "v"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_reg_read(ctx, {regN});\n")
-elif regid in {"d", "e", "x", "y"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_reg_write(ctx, {regN}, {predicated});\n")
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "P":
-if regid in {"s", "t", "u", "v"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_pred_read(ctx, {regN});\n")
-elif regid in {"d", "e", "x"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_pred_write(ctx, {regN});\n")
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "C":
-if regid == "ss":
-f.write(
-f"const int {regN} = insn->regno[{regno}] "
-"+ HEX_REG_SA0;\n"
-)
-f.write(f"ctx_log_reg_read_pair(ctx, {regN});\n")
-elif regid == "dd":
-f.write(f"const int {regN} = insn->regno[{regno}] " "+ 
HEX_REG_SA0;\n")
-f.write(f"ctx_log_reg_write_pair(ctx, {regN}, 
{predicated});\n")
-elif regid == "s":
-f.write(
-f"const int {regN} = insn->regno[{regno}] "
-"+ HEX_REG_SA0;\n"
-)
-f.write(f"ctx_log_reg_read(ctx, {regN});\n")
-elif regid == "d":
-f.write(f"const int {regN} = insn->regno[{regno}] " "+ 
HEX_REG_SA0;\n")
-f.write(f"ctx_log_reg_write(ctx, {regN}, {predicated});\n")
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "M":
-if regid == "u":
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_reg_read(ctx, {regN});\n")
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "V":
-newv = "EXT_DFL"
-if hex_common.is_new_result(tag):
-newv = "EXT_NEW"
-elif hex_common.is_tmp_result(tag):
-newv = "EXT_TMP"
-if regid in {"dd", "xx"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(
-f"ctx_log_vreg_write_pair(ctx, {regN}, {newv}, " 
f"{predicated});\n"
-)
-elif regid in {"uu", "vv"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_vreg_read_pair(ctx, {regN});\n")
-elif regid in {"s", "u", "v", "w"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_vreg_read(ctx, {regN});\n")
-elif regid in {"d", "x", "y"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_vreg_write(ctx, {regN}, {newv}, " 
f"{predicated});\n")
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "Q":
-if regid in {"d", "e", "x"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_qreg_write(ctx, {regN});\n")
-elif regid in {"s", "t", "u", "v"}:
-f.write(f"const int {regN} = insn->regno[{regno}];\n")
-f.write(f"ctx_log_qreg_read(ctx, {regN});\n")
-else:
-hex_common.bad_register(regtype, regid)
-elif regtype == "G":
-if regid in {"dd"}:
-f.wri

[PATCH v2 5/9] Hexagon (target/hexagon) Make generators object oriented - gen_idef_parser_funcs

2023-12-10 Thread Taylor Simpson
Signed-off-by: Taylor Simpson 
---
 target/hexagon/gen_idef_parser_funcs.py | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/target/hexagon/gen_idef_parser_funcs.py 
b/target/hexagon/gen_idef_parser_funcs.py
index f4518e653f..550a48cb7b 100644
--- a/target/hexagon/gen_idef_parser_funcs.py
+++ b/target/hexagon/gen_idef_parser_funcs.py
@@ -46,6 +46,7 @@ def main():
 hex_common.read_semantics_file(sys.argv[1])
 hex_common.read_attribs_file(sys.argv[2])
 hex_common.calculate_attribs()
+hex_common.init_registers()
 tagregs = hex_common.get_tagregs()
 tagimms = hex_common.get_tagimms()
 
@@ -132,22 +133,9 @@ def main():
 
 arguments = []
 for regtype, regid in regs:
-prefix = "in " if hex_common.is_read(regid) else ""
-
-is_pair = hex_common.is_pair(regid)
-is_single_old = hex_common.is_single(regid) and 
hex_common.is_old_val(
-regtype, regid, tag
-)
-is_single_new = hex_common.is_single(regid) and 
hex_common.is_new_val(
-regtype, regid, tag
-)
-
-if is_pair or is_single_old:
-arguments.append(f"{prefix}{regtype}{regid}V")
-elif is_single_new:
-arguments.append(f"{prefix}{regtype}{regid}N")
-else:
-hex_common.bad_register(regtype, regid)
+reg = hex_common.get_register(tag, regtype, regid)
+prefix = "in " if reg.is_read() else ""
+arguments.append(f"{prefix}{reg.reg_tcg()}")
 
 for immlett, bits, immshift in imms:
 arguments.append(hex_common.imm_name(immlett))
-- 
2.34.1




[PATCH v2 8/9] Hexagon (target/hexagon) Remove unused WRITES_PRED_REG attribute

2023-12-10 Thread Taylor Simpson
This is the only remaining use of the is_written function.  We will
remove it in the subsequent commit.

Signed-off-by: Taylor Simpson 
---
 target/hexagon/attribs_def.h.inc |  1 -
 target/hexagon/hex_common.py | 11 ---
 2 files changed, 12 deletions(-)

diff --git a/target/hexagon/attribs_def.h.inc b/target/hexagon/attribs_def.h.inc
index 21d457fa4a..87942d46f4 100644
--- a/target/hexagon/attribs_def.h.inc
+++ b/target/hexagon/attribs_def.h.inc
@@ -117,7 +117,6 @@ DEF_ATTRIB(IMPLICIT_READS_P1, "Reads the P1 register", "", 
"")
 DEF_ATTRIB(IMPLICIT_READS_P2, "Reads the P2 register", "", "")
 DEF_ATTRIB(IMPLICIT_READS_P3, "Reads the P3 register", "", "")
 DEF_ATTRIB(IMPLICIT_WRITES_USR, "May write USR", "", "")
-DEF_ATTRIB(WRITES_PRED_REG, "Writes a predicate register", "", "")
 DEF_ATTRIB(COMMUTES, "The operation is communitive", "", "")
 DEF_ATTRIB(DEALLOCRET, "dealloc_return", "", "")
 DEF_ATTRIB(DEALLOCFRAME, "deallocframe", "", "")
diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index 4565dd1953..ca5e9630c1 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -94,10 +94,6 @@ def is_cond_call(tag):
 def calculate_attribs():
 add_qemu_macro_attrib("fREAD_PC", "A_IMPLICIT_READS_PC")
 add_qemu_macro_attrib("fTRAP", "A_IMPLICIT_READS_PC")
-add_qemu_macro_attrib("fWRITE_P0", "A_WRITES_PRED_REG")
-add_qemu_macro_attrib("fWRITE_P1", "A_WRITES_PRED_REG")
-add_qemu_macro_attrib("fWRITE_P2", "A_WRITES_PRED_REG")
-add_qemu_macro_attrib("fWRITE_P3", "A_WRITES_PRED_REG")
 add_qemu_macro_attrib("fSET_OVERFLOW", "A_IMPLICIT_WRITES_USR")
 add_qemu_macro_attrib("fSET_LPCFG", "A_IMPLICIT_WRITES_USR")
 add_qemu_macro_attrib("fLOAD", "A_SCALAR_LOAD")
@@ -122,13 +118,6 @@ def calculate_attribs():
 continue
 macro = macros[macname]
 attribdict[tag] |= set(macro.attribs)
-# Figure out which instructions write predicate registers
-tagregs = get_tagregs()
-for tag in tags:
-regs = tagregs[tag]
-for regtype, regid in regs:
-if regtype == "P" and is_written(regid):
-attribdict[tag].add("A_WRITES_PRED_REG")
 # Mark conditional jumps and calls
 # Not all instructions are properly marked with A_CONDEXEC
 for tag in tags:
-- 
2.34.1




[PATCH v2 4/9] Hexagon (target/hexagon) Make generators object oriented - gen_helper_funcs

2023-12-10 Thread Taylor Simpson
Signed-off-by: Taylor Simpson 
---
 target/hexagon/gen_helper_funcs.py | 368 +
 target/hexagon/hex_common.py   |  48 +++-
 2 files changed, 103 insertions(+), 313 deletions(-)

diff --git a/target/hexagon/gen_helper_funcs.py 
b/target/hexagon/gen_helper_funcs.py
index ce21d3b688..9cc3d69c49 100755
--- a/target/hexagon/gen_helper_funcs.py
+++ b/target/hexagon/gen_helper_funcs.py
@@ -23,181 +23,14 @@
 import hex_common
 
 
-##
-## Helpers for gen_helper_function
-##
-def gen_decl_ea(f):
-f.write("uint32_t EA;\n")
-
-
-def gen_helper_return_type(f, regtype, regid, regno):
-if regno > 1:
-f.write(", ")
-f.write("int32_t")
-
-
-def gen_helper_return_type_pair(f, regtype, regid, regno):
-if regno > 1:
-f.write(", ")
-f.write("int64_t")
-
-
-def gen_helper_arg(f, regtype, regid, regno):
-if regno > 0:
-f.write(", ")
-f.write(f"int32_t {regtype}{regid}V")
-
-
-def gen_helper_arg_new(f, regtype, regid, regno):
-if regno >= 0:
-f.write(", ")
-f.write(f"int32_t {regtype}{regid}N")
-
-
-def gen_helper_arg_pair(f, regtype, regid, regno):
-if regno >= 0:
-f.write(", ")
-f.write(f"int64_t {regtype}{regid}V")
-
-
-def gen_helper_arg_ext(f, regtype, regid, regno):
-if regno > 0:
-f.write(", ")
-f.write(f"void *{regtype}{regid}V_void")
-
-
-def gen_helper_arg_ext_pair(f, regtype, regid, regno):
-if regno > 0:
-f.write(", ")
-f.write(f"void *{regtype}{regid}V_void")
-
-
-def gen_helper_arg_opn(f, regtype, regid, i, tag):
-if hex_common.is_pair(regid):
-if hex_common.is_hvx_reg(regtype):
-gen_helper_arg_ext_pair(f, regtype, regid, i)
-else:
-gen_helper_arg_pair(f, regtype, regid, i)
-elif hex_common.is_single(regid):
-if hex_common.is_old_val(regtype, regid, tag):
-if hex_common.is_hvx_reg(regtype):
-gen_helper_arg_ext(f, regtype, regid, i)
-else:
-gen_helper_arg(f, regtype, regid, i)
-elif hex_common.is_new_val(regtype, regid, tag):
-gen_helper_arg_new(f, regtype, regid, i)
-else:
-hex_common.bad_register(regtype, regid)
-else:
-hex_common.bad_register(regtype, regid)
-
-
-def gen_helper_arg_imm(f, immlett):
-f.write(f", int32_t {hex_common.imm_name(immlett)}")
-
-
-def gen_helper_dest_decl(f, regtype, regid, regno, subfield=""):
-f.write(f"int32_t {regtype}{regid}V{subfield} = 0;\n")
-
-
-def gen_helper_dest_decl_pair(f, regtype, regid, regno, subfield=""):
-f.write(f"int64_t {regtype}{regid}V{subfield} = 0;\n")
-
-
-def gen_helper_dest_decl_ext(f, regtype, regid):
-if regtype == "Q":
-f.write(
-f"/* {regtype}{regid}V is *(MMQReg *)" 
f"({regtype}{regid}V_void) */\n"
-)
-else:
-f.write(
-f"/* {regtype}{regid}V is *(MMVector *)"
-f"({regtype}{regid}V_void) */\n"
-)
-
-
-def gen_helper_dest_decl_ext_pair(f, regtype, regid, regno):
-f.write(
-f"/* {regtype}{regid}V is *(MMVectorPair *))"
-f"{regtype}{regid}V_void) */\n"
-)
-
-
-def gen_helper_dest_decl_opn(f, regtype, regid, i):
-if hex_common.is_pair(regid):
-if hex_common.is_hvx_reg(regtype):
-gen_helper_dest_decl_ext_pair(f, regtype, regid, i)
-else:
-gen_helper_dest_decl_pair(f, regtype, regid, i)
-elif hex_common.is_single(regid):
-if hex_common.is_hvx_reg(regtype):
-gen_helper_dest_decl_ext(f, regtype, regid)
-else:
-gen_helper_dest_decl(f, regtype, regid, i)
-else:
-hex_common.bad_register(regtype, regid)
-
-
-def gen_helper_src_var_ext(f, regtype, regid):
-if regtype == "Q":
-f.write(
-f"/* {regtype}{regid}V is *(MMQReg *)" 
f"({regtype}{regid}V_void) */\n"
-)
-else:
-f.write(
-f"/* {regtype}{regid}V is *(MMVector *)"
-f"({regtype}{regid}V_void) */\n"
-)
-
-
-def gen_helper_src_var_ext_pair(f, regtype, regid, regno):
-f.write(
-f"/* {regtype}{regid}V{regno} is *(MMVectorPair *)"
-f"({regtype}{regid}V{regno}_void) */\n"
-)
-
-
-def gen_helper_return(f, regtype, regid, regno):
-f.write(f"return {regtype}{regid}V;\n")
-
-
-def gen_helper_return_pair(f, regtype, regid, regno):
-f.write(f"return {regtype}{regid}V;\n")
-
-
-def gen_helper_dst_write_ext(f, regtype, regid):
-return
-
-
-def gen_helper_dst_write_ext_pair(f, regtype, regid):
-return
-
-
-def gen_helper_return_opn(f, regtype, regid, i):
-if hex_common.is_pair(regid):
-if hex_common.is_hvx_reg(regtype):
-gen_helper_dst_write_ext_pair(f, regtype, regid)
-else:
-gen_helper_return_pair(f, regtype, regid, i)
-elif hex_common.is_single(regid):
-if hex_common.is_hvx_reg(regtype):
-g

Re: [PATCH v4] arm/kvm: Enable support for KVM_ARM_VCPU_PMU_V3_FILTER

2023-12-10 Thread Gavin Shan

Hi Shaoqin,

On 12/7/23 20:36, Shaoqin Huang wrote:

The KVM_ARM_VCPU_PMU_V3_FILTER provide the ability to let the VMM decide

 ^^^
 provides

which PMU events are provided to the guest. Add a new option
`pmu-filter` as -accel sub-option to set the PMU Event Filtering.

  ^^
  '-accel kvm'


Without the filter, the KVM will expose all events from the host to
guest by default.


Without the filter, all PMU events are exposed from host to guest by
default.



The `pmu-filter` has such format:

   pmu-filter="{A,D}:start-end[;{A,D}:start-end...]"

The A means "allow" and D means "deny", start is the first event of the
range and the end is the last one. The first registered range defines
the global policy(global ALLOW if the first @action is DENY, global DENY
if the first @action is ALLOW). The start and end only support hex
format now. For example:

   pmu-filter="A:0x11-0x11;A:0x23-0x3a;D:0x30-0x30"

Since the first action is allow, we have a global deny policy. It
will allow event 0x11 (The cycle counter), events 0x23 to 0x3a is
also allowed except the event 0x30 is denied, and all the other events
are disallowed.



The section explaining how 'pmu-filter' is used has been well documented
in qemu-options.hx. So it can be dropped Instead, it can be mentioned in
the commit message, something like below.

The usage of the new sub-option can be found from the updated document
(qemu-options.hx).


Here is an real example shows how to use the PMU Event Filtering, when

  ^^^
  an example

we launch a guest by use kvm, add such command line:

   # qemu-system-aarch64 \
-accel kvm,pmu-filter="D:0x11-0x11"

Since the first action is deny, we have a global allow policy. This
disables the filtering of the cycle counter (event 0x11 being CPU_CYCLES).

And then in guest, use the perf to count the cycle:

   # perf stat sleep 1

Performance counter stats for 'sleep 1':

   1.22 msec task-clock   #0.001 CPUs 
utilized
  1  context-switches #  820.695 /sec
  0  cpu-migrations   #0.000 /sec
 55  page-faults  #   45.138 K/sec
  cycles
1128954  instructions
 227031  branches #  186.323 M/sec
   8686  branch-misses#3.83% of all 
branches

1.002492480 seconds time elapsed

0.001752000 seconds user
0.0 seconds sys

As we can see, the cycle counter has been disabled in the guest, but
other pmu events are still work.

Signed-off-by: Shaoqin Huang 
---
v3->v4:
   - Fix the wrong check for pmu_filter_init.[Sebastian]
   - Fix multiple alignment issue.   [Gavin]
   - Report error by warn_report() instead of error_report(), and don't use
   abort() since the PMU Event Filter is an add-on and best-effort feature.
 [Gavin]
   - Add several missing {  } for single line of code.   [Gavin]
   - Use the g_strsplit() to replace strtok().   [Gavin]

v2->v3:
   - Improve commits message, use kernel doc wording, add more explaination on
 filter example, fix some typo error.[Eric]
   - Add g_free() in kvm_arch_set_pmu_filter() to prevent memory leak. [Eric]
   - Add more precise error message report.  [Eric]
   - In options doc, add pmu-filter rely on KVM_ARM_VCPU_PMU_V3_FILTER support 
in
 KVM.[Eric]

v1->v2:
   - Add more description for allow and deny meaning in
 commit message. [Sebastian]
   - Small improvement.  [Sebastian]

v2: https://lore.kernel.org/all/20231117060838.39723-1-shahu...@redhat.com/
v1: https://lore.kernel.org/all/20231113081713.153615-1-shahu...@redhat.com/
---
  include/sysemu/kvm_int.h |  1 +
  qemu-options.hx  | 21 +++
  target/arm/kvm.c | 23 
  target/arm/kvm64.c   | 75 
  4 files changed, 120 insertions(+)


I would simplify the commit message like below, for your reference.

---

The KVM_ARM_VCPU_PMU_V3_FILTER provides the ability letting the VMM
to decide which PMU events are present to the guest. Add a new option
`pmu-filter` as '-accel kvm' sub-option to set the PMU Event Filtering.
The usage of the new sub-option can be found from the updated docuement
(qemu-options.hx).

Here is an example shows how to use the PMU Event Filtering.

  # qemu-system-aarch64 -accel kvm,pmu-filter="D:0x11-0x11"

Since the first action is deny, we have a global allow policy. This
disables the filtering of the cycle counter (event 0x11). In the
guest, we use the perf to count the CPU

Re: [PATCH] target/i386: Give IRQs a chance when resetting HF_INHIBIT_IRQ_MASK

2023-12-10 Thread Ruihan Li
On Mon, Dec 11, 2023 at 03:01:48AM +0800, Ruihan Li wrote:
> [ .. ]
> 
> This problem is caused because the previous code may choose not to end
> the TB even if the HF_INHIBIT_IRQ_MASK has just been reset (e.g., in the
> case where the RET instruction is immediately followed by the STI
> instruction), so that IRQs may not have a change to trigger.

There is a typo. I mean "in the case where the STI instruction is
immediately followed by the RET instruction", not the other way around.

I'll send a V2 patch if necessary, but let's hear from others first.

Thanks,
Ruihan Li




Re: [PATCH RFC v2 00/12] virtio-net: add support for SR-IOV emulation

2023-12-10 Thread Jason Wang
On Sun, Dec 10, 2023 at 12:06 PM Akihiko Odaki  wrote:
>
> Introduction
> 
>
> This series is based on the RFC series submitted by Yui Washizu[1].
> See also [2] for the context.
>
> This series enables SR-IOV emulation for virtio-net. It is useful
> to test SR-IOV support on the guest, or to expose several vDPA devices
> in a VM. vDPA devices can also provide L2 switching feature for
> offloading though it is out of scope to allow the guest to configure
> such a feature.
>
> The PF side code resides in virtio-pci. The VF side code resides in
> the PCI common infrastructure, but it is restricted to work only for
> virtio-net-pci because of lack of validation.
>
> User Interface
> --
>
> A user can configure a SR-IOV capable virtio-net device by adding
> virtio-net-pci functions to a bus. Below is a command line example:
>   -netdev user,id=n -netdev user,id=o
>   -netdev user,id=p -netdev user,id=q
>   -device pcie-root-port,id=b
>   -device virtio-net-pci,bus=b,addr=0x0.0x3,netdev=q,sriov-pf=f
>   -device virtio-net-pci,bus=b,addr=0x0.0x2,netdev=p,sriov-pf=f
>   -device virtio-net-pci,bus=b,addr=0x0.0x1,netdev=o,sriov-pf=f
>   -device virtio-net-pci,bus=b,addr=0x0.0x0,netdev=n,id=f
>
> The VFs specify the paired PF with "sriov-pf" property. The PF must be
> added after all VFs. It is user's responsibility to ensure that VFs have
> function numbers larger than one of the PF, and the function numbers
> have a consistent stride.

This seems not user friendly. Any reason we can't just allow user to
specify the stride here?

Btw, I vaguely remember qemu allows the params to be accepted as a
list. If this is true, we can accept a list of netdev here?

>
> Keeping VF instances
> 
>
> A problem with SR-IOV emulation is that it needs to hotplug the VFs as
> the guest requests. Previously, this behavior was implemented by
> realizing and unrealizing VFs at runtime. However, this strategy does
> not work well for the proposed virtio-net emulation; in this proposal,
> device options passed in the command line must be maintained as VFs
> are hotplugged, but they are consumed when the machine starts and not
> available after that, which makes realizing VFs at runtime impossible.

Could we store the device options in the PF?

Thanks

>
> As an strategy alternative to runtime realization/unrealization, this
> series proposes to reuse the code to power down PCI Express devices.
> When a PCI Express device is powered down, it will be hidden from the
> guest but will be kept realized. This effectively implements the
> behavior we need for the SR-IOV emulation.
>
> Summary
> ---
>
> Patch [1, 5] refactors the PCI infrastructure code.
> Patch [6, 10] adds user-created SR-IOV VF infrastructure.
> Patch 11 makes virtio-pci work as SR-IOV PF for user-created VFs.
> Patch 12 allows user to create SR-IOV VFs with virtio-net-pci.
>
> [1] 
> https://patchew.org/QEMU/1689731808-3009-1-git-send-email-yui.wash...@gmail.com/
> [2] 
> https://lore.kernel.org/all/5d46f455-f530-4e5e-9ae7-13a2297d4...@daynix.com/
>
> Co-developed-by: Yui Washizu 
> Signed-off-by: Akihiko Odaki 
> ---
> Changes in v2:
> - Changed to keep VF instances.
> - Link to v1: 
> https://lore.kernel.org/r/20231202-sriov-v1-0-32b3570f7...@daynix.com
>
> ---
> Akihiko Odaki (12):
>   hw/pci: Initialize PCI multifunction after realization
>   hw/pci: Determine if rombar is explicitly enabled
>   hw/pci: Do not add ROM BAR for SR-IOV VF
>   vfio: Avoid inspecting option QDict for rombar
>   hw/qdev: Remove opts member
>   pcie_sriov: Reuse SR-IOV VF device instances
>   pcie_sriov: Release VFs failed to realize
>   pcie_sriov: Ensure PF and VF are mutually exclusive
>   pcie_sriov: Check PCI Express for SR-IOV PF
>   pcie_sriov: Allow user to create SR-IOV device
>   virtio-pci: Implement SR-IOV PF
>   virtio-net: Implement SR-IOV VF
>
>  docs/pcie_sriov.txt |   8 +-
>  include/hw/pci/pci.h|   2 +-
>  include/hw/pci/pci_device.h |  13 +-
>  include/hw/pci/pcie_sriov.h |  25 ++-
>  include/hw/qdev-core.h  |   4 -
>  hw/core/qdev.c  |   1 -
>  hw/net/igb.c|   3 +-
>  hw/nvme/ctrl.c  |   3 +-
>  hw/pci/pci.c|  98 +++-
>  hw/pci/pci_host.c   |   4 +-
>  hw/pci/pcie.c   |   4 +-
>  hw/pci/pcie_sriov.c | 360 
> +---
>  hw/vfio/pci.c   |   3 +-
>  hw/virtio/virtio-net-pci.c  |   1 +
>  hw/virtio/virtio-pci.c  |   7 +
>  system/qdev-monitor.c   |  12 +-
>  16 files changed, 395 insertions(+), 153 deletions(-)
> ---
> base-commit: 4705fc0c8511d073bee4751c3c974aab2b10a970
> change-id: 20231202-sriov-9402fb262be8
>
> Best regards,
> --
> Akihiko Odaki 
>




Re: [PATCH RFC v2 00/12] virtio-net: add support for SR-IOV emulation

2023-12-10 Thread Akihiko Odaki

On 2023/12/11 11:52, Jason Wang wrote:

On Sun, Dec 10, 2023 at 12:06 PM Akihiko Odaki  wrote:


Introduction


This series is based on the RFC series submitted by Yui Washizu[1].
See also [2] for the context.

This series enables SR-IOV emulation for virtio-net. It is useful
to test SR-IOV support on the guest, or to expose several vDPA devices
in a VM. vDPA devices can also provide L2 switching feature for
offloading though it is out of scope to allow the guest to configure
such a feature.

The PF side code resides in virtio-pci. The VF side code resides in
the PCI common infrastructure, but it is restricted to work only for
virtio-net-pci because of lack of validation.

User Interface
--

A user can configure a SR-IOV capable virtio-net device by adding
virtio-net-pci functions to a bus. Below is a command line example:
   -netdev user,id=n -netdev user,id=o
   -netdev user,id=p -netdev user,id=q
   -device pcie-root-port,id=b
   -device virtio-net-pci,bus=b,addr=0x0.0x3,netdev=q,sriov-pf=f
   -device virtio-net-pci,bus=b,addr=0x0.0x2,netdev=p,sriov-pf=f
   -device virtio-net-pci,bus=b,addr=0x0.0x1,netdev=o,sriov-pf=f
   -device virtio-net-pci,bus=b,addr=0x0.0x0,netdev=n,id=f

The VFs specify the paired PF with "sriov-pf" property. The PF must be
added after all VFs. It is user's responsibility to ensure that VFs have
function numbers larger than one of the PF, and the function numbers
have a consistent stride.


This seems not user friendly. Any reason we can't just allow user to
specify the stride here?


It should be possible to assign addr automatically without requiring 
user to specify the stride. I'll try that in the next version.




Btw, I vaguely remember qemu allows the params to be accepted as a
list. If this is true, we can accept a list of netdev here?


Yes, rocker does that. But the problem is not just about getting 
parameters needed for VFs, which I forgot to mention in the cover letter 
and will explain below.






Keeping VF instances


A problem with SR-IOV emulation is that it needs to hotplug the VFs as
the guest requests. Previously, this behavior was implemented by
realizing and unrealizing VFs at runtime. However, this strategy does
not work well for the proposed virtio-net emulation; in this proposal,
device options passed in the command line must be maintained as VFs
are hotplugged, but they are consumed when the machine starts and not
available after that, which makes realizing VFs at runtime impossible.


Could we store the device options in the PF?


I wrote it's to store the device options, but the problem is actually 
more about realizing VFs at runtime instead of at the initialization time.


Realizing VFs at runtime have two major problems. One is that it delays 
the validations of options; invalid options will be noticed when the 
guest requests to realize VFs. netdevs also warn that they are not used 
at initialization time, not knowing that they will be used by VFs later. 
References to other QEMU objects in the option may also die before VFs 
are realized.


The other problem is that QEMU cannot interact with the unrealized VFs. 
For example, if you type "device_add virtio-net-pci,id=vf,sriov-pf=pf" 
in HMP, you will expect "device_del vf" works, but it's hard to 
implement such behaviors with unrealized VFs.


I was first going to compromise and allow such quirky behaviors, but I 
realized such a compromise is unnecessary if we reuse the PCI power down 
logic so I wrote v2.


Regards,
Akihiko Odaki



[PATCH v6] ui/cocoa: Use NSWindow's ability to resize

2023-12-10 Thread Akihiko Odaki
This change brings two new features:
- The window will be resizable if "Zoom To Fit" is eanbled
- The window can be made full screen by clicking full screen button
  provided by the platform. (The left-top green button.)

Signed-off-by: Akihiko Odaki 
---
V5 -> V6:
  Rebased.
---
 ui/cocoa.m | 542 +
 1 file changed, 258 insertions(+), 284 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index cd069da696..302e4f76be 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -99,12 +99,10 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 static DisplayChangeListener dcl = {
 .ops = &dcl_ops,
 };
-static int last_buttons;
 static int cursor_hide = 1;
 static int left_command_key_enabled = 1;
 static bool swap_opt_cmd;
 
-static bool stretch_video;
 static NSTextField *pauseLabel;
 
 static bool allow_events;
@@ -304,20 +302,17 @@ static void handleAnyDeviceErrors(Error * err)
 */
 @interface QemuCocoaView : NSView
 {
+NSTrackingArea *trackingArea;
 QEMUScreen screen;
-NSWindow *fullScreenWindow;
-float cx,cy,cw,ch,cdx,cdy;
 pixman_image_t *pixman_image;
 QKbdState *kbd;
 BOOL isMouseGrabbed;
-BOOL isFullscreen;
 BOOL isAbsoluteEnabled;
 CFMachPortRef eventsTap;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
 - (void) ungrabMouse;
-- (void) toggleFullScreen:(id)sender;
 - (void) setFullGrab:(id)sender;
 - (void) handleMonitorInput:(NSEvent *)event;
 - (bool) handleEvent:(NSEvent *)event;
@@ -333,8 +328,6 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
  */
 - (BOOL) isMouseGrabbed;
 - (BOOL) isAbsoluteEnabled;
-- (float) cdx;
-- (float) cdy;
 - (QEMUScreen) gscreen;
 - (void) raiseAllKeys;
 @end
@@ -392,46 +385,43 @@ - (BOOL) isOpaque
 return YES;
 }
 
-- (BOOL) screenContainsPoint:(NSPoint) p
+- (void) removeTrackingRect
 {
-return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
+if (trackingArea) {
+[self removeTrackingArea:trackingArea];
+[trackingArea release];
+trackingArea = nil;
+}
 }
 
-/* Get location of event and convert to virtual screen coordinate */
-- (CGPoint) screenLocationOfEvent:(NSEvent *)ev
+- (void) frameUpdated
 {
-NSWindow *eventWindow = [ev window];
-// XXX: Use CGRect and -convertRectFromScreen: to support macOS 10.10
-CGRect r = CGRectZero;
-r.origin = [ev locationInWindow];
-if (!eventWindow) {
-if (!isFullscreen) {
-return [[self window] convertRectFromScreen:r].origin;
-} else {
-CGPoint locationInSelfWindow = [[self window] 
convertRectFromScreen:r].origin;
-CGPoint loc = [self convertPoint:locationInSelfWindow 
fromView:nil];
-if (stretch_video) {
-loc.x /= cdx;
-loc.y /= cdy;
-}
-return loc;
-}
-} else if ([[self window] isEqual:eventWindow]) {
-if (!isFullscreen) {
-return r.origin;
-} else {
-CGPoint loc = [self convertPoint:r.origin fromView:nil];
-if (stretch_video) {
-loc.x /= cdx;
-loc.y /= cdy;
-}
-return loc;
-}
-} else {
-return [[self window] convertRectFromScreen:[eventWindow 
convertRectToScreen:r]].origin;
+[self removeTrackingRect];
+
+if ([self window]) {
+NSTrackingAreaOptions options = NSTrackingActiveInKeyWindow |
+NSTrackingMouseEnteredAndExited |
+NSTrackingMouseMoved;
+trackingArea = [[NSTrackingArea alloc] initWithRect:[self frame]
+options:options
+  owner:self
+   userInfo:nil];
+[self addTrackingArea:trackingArea];
+[self updateUIInfo];
 }
 }
 
+- (void) viewDidMoveToWindow
+{
+[self resizeWindow];
+[self frameUpdated];
+}
+
+- (void) viewWillMoveToWindow:(NSWindow *)newWindow
+{
+[self removeTrackingRect];
+}
+
 - (void) hideCursor
 {
 if (!cursor_hide) {
@@ -494,13 +484,14 @@ - (void) drawRect:(NSRect) rect
 int i;
 CGImageRef clipImageRef;
 CGRect clipRect;
+CGFloat d = (CGFloat)h / [self frame].size.height;
 
 [self getRectsBeingDrawn:&rectList count:&rectCount];
 for (i = 0; i < rectCount; i++) {
-clipRect.origin.x = rectList[i].origin.x / cdx;
-clipRect.origin.y = (float)h - (rectList[i].origin.y + 
rectList[i].size.height) / cdy;
-clipRect.size.width = rectList[i].size.width / cdx;
-clipRect.size.height = rectList[i].size.height / cdy;
+clipRect.origin.x = rectList[i].origin.x * d;
+clipRect.origin.y = (float)h - (rectList[i].origin.y + 
rectList[i].size.height) * d;
+  

RE: [PATCH for-9.0 01/10] vfio/spapr: Extend VFIOIOMMUOps with a release handler

2023-12-10 Thread Duan, Zhenzhong
Hi Cédric,

>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 01/10] vfio/spapr: Extend VFIOIOMMUOps with a
>release handler
>
>This allows to abstract a bit more the sPAPR IOMMU support in the
>legacy IOMMU backend.
>
>Signed-off-by: Cédric Le Goater 

Reviewed-by: Zhenzhong Duan 

Thanks
Zhenzhong


RE: [PATCH for-9.0 02/10] vfio/container: Introduce vfio_legacy_setup() for further cleanups

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 02/10] vfio/container: Introduce vfio_legacy_setup()
>for further cleanups
>
>This will help subsequent patches to unify the initialization of type1
>and sPAPR IOMMU backends.
>
>Signed-off-by: Cédric Le Goater 

Reviewed-by: Zhenzhong Duan 

Thanks
Zhenzhong



Re: [PATCH] pc: q35: Bump max_cpus to 4096

2023-12-10 Thread Thomas Huth

On 08/12/2023 13.26, Ani Sinha wrote:

Since commit f10a570b093e6 ("KVM: x86: Add CONFIG_KVM_MAX_NR_VCPUS to allow up to 
4096 vCPUs")
Linux kernel can support upto a maximum number of 4096 vCPUS when MAXSMP is
enabled in the kernel. So bump up the max_cpus value for q35 machines versions
8.3 and newer to 4096. Older q35 machines versions 8.2 and older continue to
support 1024 maximum vcpus as before.

If KVM is not able to support the specified number of vcpus, QEMU would
return the following error messages:

$ ./qemu-system-x86_64 -cpu host -accel kvm -machine q35 -smp 4096
qemu-system-x86_64: -accel kvm: warning: Number of SMP cpus requested (4096) 
exceeds the recommended cpus supported by KVM (12)
Number of SMP cpus requested (4096) exceeds the maximum cpus supported by KVM 
(1024)

Cc: Daniel P. Berrangé 
Cc: Igor Mammedov 
Cc: Michael S. Tsirkin 
Cc: Julia Suvorova 
Signed-off-by: Ani Sinha 
---
  hw/i386/pc_q35.c | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4f3e5412f6..2ed57814e1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -375,7 +375,7 @@ static void pc_q35_machine_options(MachineClass *m)
  m->default_nic = "e1000e";
  m->default_kernel_irqchip_split = false;
  m->no_floppy = 1;
-m->max_cpus = 1024;
+m->max_cpus = 4096;
  m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
  machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
  machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
@@ -383,12 +383,22 @@ static void pc_q35_machine_options(MachineClass *m)
  machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
  }
  
-static void pc_q35_8_2_machine_options(MachineClass *m)

+static void pc_q35_8_3_machine_options(MachineClass *m)


 Hi Ani!

The next QEMU version after 8.2 is 9.0. Maybe best if you rebase your patch 
on top of:


 https://lore.kernel.org/qemu-devel/20231120094259.1191804-1-coh...@redhat.com/

  Thomas




RE: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps
>under vfio_init_container()
>
>vfio_init_container() already defines the IOMMU type of the container.
>Do the same for the VFIOIOMMUOps struct. This prepares ground for the
>following patches that will deduce the associated VFIOIOMMUOps struct
>from the IOMMU type.
>
>Signed-off-by: Cédric Le Goater 
>---
> hw/vfio/container.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8
>879cb98e686afccc 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer
>*container,
> }
>
> static int vfio_init_container(VFIOContainer *container, int group_fd,
>-   Error **errp)
>+   VFIOAddressSpace *space, Error **errp)
> {
> int iommu_type, ret;
>
>@@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer
>*container, int group_fd,
> }
>
> container->iommu_type = iommu_type;
>+vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);

Reviewed-by: Zhenzhong Duan 

Not related to this patch, not clear if it's deserved to rename
vfio_container_init as vfio_bcontainer_init to distinguish
with vfio_init_container.

Thanks
Zhenzhong

> return 0;
> }
>
>@@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup *group,
>AddressSpace *as,
> container = g_malloc0(sizeof(*container));
> container->fd = fd;
> bcontainer = &container->bcontainer;
>-vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>
>-ret = vfio_init_container(container, group->fd, errp);
>+ret = vfio_init_container(container, group->fd, space, errp);
> if (ret) {
> goto free_container_exit;
> }
>--
>2.43.0



Re: [PATCH v3 3/3] cpu, softmmu/vl.c: Change parsing of -cpu argument to allow -cpu cpu, help to print options for the CPU type similar to how the '-device' option works.

2023-12-10 Thread Dinah B
Hi,

Due to extracting CPU features via a qmp command, it only works on
qemu-system-* builds.
Building qmp for non system builds strikes me as extreme overkill so I need
a way to exclude this from non system builds.
Or maybe there's a way to disentangle querying CPU features independent
from the qom or qmp based data structures it's currently intertwined with.

Thanks,
-Dinah

On Tue, Nov 14, 2023 at 12:44 PM Markus Armbruster 
wrote:

> Dinah B  writes:
>
> > Hi,
> >
> > Is there a way to distinguish between qemu-system-* vs qemu-* builds?
> > At first I thought #CONFIG_LINUX_USER might be it but not all non-mmu
> > builds set this.
>
> What are you trying to accomplish?
>
>


RE: [PATCH for-9.0 04/10] vfio/container: Introduce a VFIOIOMMU QOM interface

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 04/10] vfio/container: Introduce a VFIOIOMMU
>QOM interface
>
>Simply transform the VFIOIOMMUOps struct in an InterfaceClass and do
>some initial name replacements. Next changes will start converting
>VFIOIOMMUOps.
>
>Signed-off-by: Cédric Le Goater 
>---
> include/hw/vfio/vfio-container-base.h | 18 ++
> hw/vfio/common.c  |  2 +-
> hw/vfio/container-base.c  | 12 +++-
> hw/vfio/pci.c |  2 +-
> 4 files changed, 27 insertions(+), 7 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>5c9594b6c77681e5593236e711e7e391e5f2bdff..81d49fe562d3840859096
>dd8a62ac38d62314939 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -16,7 +16,8 @@
> #include "exec/memory.h"
>
> typedef struct VFIODevice VFIODevice;
>-typedef struct VFIOIOMMUOps VFIOIOMMUOps;
>+typedef struct VFIOIOMMUClass VFIOIOMMUClass;
>+#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
>
> typedef struct {
> unsigned long *bitmap;
>@@ -34,7 +35,7 @@ typedef struct VFIOAddressSpace {
>  * This is the base object for vfio container backends
>  */
> typedef struct VFIOContainerBase {
>-const VFIOIOMMUOps *ops;
>+const VFIOIOMMUClass *ops;
> VFIOAddressSpace *space;
> MemoryListener listener;
> Error *error;
>@@ -88,10 +89,19 @@ int vfio_container_query_dirty_bitmap(const
>VFIOContainerBase *bcontainer,
>
> void vfio_container_init(VFIOContainerBase *bcontainer,
>  VFIOAddressSpace *space,
>- const VFIOIOMMUOps *ops);
>+ const VFIOIOMMUClass *ops);
> void vfio_container_destroy(VFIOContainerBase *bcontainer);
>
>-struct VFIOIOMMUOps {
>+typedef struct VFIOIOMMU VFIOIOMMU;
>+
>+#define TYPE_VFIO_IOMMU "vfio-iommu"
>+
>+#define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj),
>TYPE_VFIO_IOMMU)

Maybe this #define can be removed or you have other plans?
Otherwise, Reviewed-by: Zhenzhong Duan 

Thanks
Zhenzhong

>+DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU,
>TYPE_VFIO_IOMMU)
>+
>+struct VFIOIOMMUClass {
>+InterfaceClass parent_class;
>+
> /* basic feature */
> int (*dma_map)(const VFIOContainerBase *bcontainer,
>hwaddr iova, ram_addr_t size,
>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>index
>08a3e576725b1fc9f2f7e425375df3b827c4fe56..49dab41566f07ba7be1100f
>ed1973e028d34467c 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1503,7 +1503,7 @@ retry:
> int vfio_attach_device(char *name, VFIODevice *vbasedev,
>AddressSpace *as, Error **errp)
> {
>-const VFIOIOMMUOps *ops = &vfio_legacy_ops;
>+const VFIOIOMMUClass *ops = &vfio_legacy_ops;
>
> #ifdef CONFIG_IOMMUFD
> if (vbasedev->iommufd) {
>diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
>index
>1ffd25bbfa8bd3d404e43b96357273b95f5a0031..913ae49077c4f09b7b275
>17c1231cfbe4befb7fb 100644
>--- a/hw/vfio/container-base.c
>+++ b/hw/vfio/container-base.c
>@@ -72,7 +72,7 @@ int vfio_container_query_dirty_bitmap(const
>VFIOContainerBase *bcontainer,
> }
>
> void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace
>*space,
>- const VFIOIOMMUOps *ops)
>+ const VFIOIOMMUClass *ops)
> {
> bcontainer->ops = ops;
> bcontainer->space = space;
>@@ -99,3 +99,13 @@ void vfio_container_destroy(VFIOContainerBase
>*bcontainer)
>
> g_list_free_full(bcontainer->iova_ranges, g_free);
> }
>+
>+static const TypeInfo types[] = {
>+{
>+.name = TYPE_VFIO_IOMMU,
>+.parent = TYPE_INTERFACE,
>+.class_size = sizeof(VFIOIOMMUClass),
>+},
>+};
>+
>+DEFINE_TYPES(types)
>diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>index
>1874ec1aba987cac6cb83f86650e7a5e1968c327..d84a9e73a65de4e4c1cdaf
>65619a700bd8d6b802 100644
>--- a/hw/vfio/pci.c
>+++ b/hw/vfio/pci.c
>@@ -2488,7 +2488,7 @@ int
>vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
> static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
> {
> VFIODevice *vbasedev = &vdev->vbasedev;
>-const VFIOIOMMUOps *ops = vbasedev->bcontainer->ops;
>+const VFIOIOMMUClass *ops = vbasedev->bcontainer->ops;
>
> return ops->pci_hot_reset(vbasedev, single);
> }
>--
>2.43.0



Re: Request for New PPC Machine Supporting Multiple SMP Cores

2023-12-10 Thread Thomas Huth

On 07/12/2023 13.53, aziz tlili wrote:

Dear QEMU Team,

I hope this message finds you well. I've been a user of QEMU for well over a 
year.


I wanted to share an idea for a potential enhancement that I believe could 
benefit many users, including myself. It would be fantastic to have a new 
PPC machine model similar to the existing mac99, but with support for 
multiple SMP cores.


 Hi!

Does it have to be a mac compatible machine? You can use the "pseries" or 
the "powernv" machines already if you need SMP in a PPC guest...


 Thomas


The ability to simulate multiple SMP cores within a PPC machine environment 
would significantly enhance the capabilities of QEMU for various 
applications, testing scenarios, and development purposes. This addition 
could greatly benefit the community working on PowerPC architecture.


I understand the complexities involved in such developments but wanted to 
express the potential advantages and how this enhancement could contribute 
to expanding QEMU's capabilities.


Thank you for considering my suggestion. I would be more than happy to 
provide further details or collaborate in any way that could assist in 
making this idea a reality.


Best regards,
Aziz Tlili





RE: [PATCH for-9.0 05/10] vfio/container: Introduce a VFIOIOMMU legacy QOM interface

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 05/10] vfio/container: Introduce a VFIOIOMMU
>legacy QOM interface
>
>Convert the legacy VFIOIOMMUOps struct to the new VFIOIOMMU QOM
>interface. The set of of operations for this backend can be referenced
>with a literal typename instead of a C struct. This will simplify
>support of multiple backends.
>
>Signed-off-by: Cédric Le Goater 
>---
> include/hw/vfio/vfio-common.h |  1 -
> include/hw/vfio/vfio-container-base.h |  1 +
> hw/vfio/common.c  |  6 ++-
> hw/vfio/container.c   | 59 +++
> 4 files changed, 56 insertions(+), 11 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>index
>b8aa8a549532442a31c8e85ce385c992d84f6bd5..14c497b6b0a79466e8f56
>7aceed384ec2c75ea90 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup)
>VFIOGroupList;
> typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
> extern VFIOGroupList vfio_group_list;
> extern VFIODeviceList vfio_device_list;
>-extern const VFIOIOMMUOps vfio_legacy_ops;
> extern const VFIOIOMMUOps vfio_iommufd_ops;
> extern const MemoryListener vfio_memory_listener;
> extern int vfio_kvm_device_fd;
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>81d49fe562d3840859096dd8a62ac38d62314939..a31fd9c2e3b9a571083e
>a8987ac27e91b332c170 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -95,6 +95,7 @@ void vfio_container_destroy(VFIOContainerBase
>*bcontainer);
> typedef struct VFIOIOMMU VFIOIOMMU;
>
> #define TYPE_VFIO_IOMMU "vfio-iommu"
>+#define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>
> #define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj),
>TYPE_VFIO_IOMMU)
> DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU,
>TYPE_VFIO_IOMMU)
>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>index
>49dab41566f07ba7be1100fed1973e028d34467c..2329d0efc8c1d617f0bfee
>5283e82b295d2d477d 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1503,13 +1503,17 @@ retry:
> int vfio_attach_device(char *name, VFIODevice *vbasedev,
>AddressSpace *as, Error **errp)
> {
>-const VFIOIOMMUClass *ops = &vfio_legacy_ops;
>+const VFIOIOMMUClass *ops =
>+
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
>
> #ifdef CONFIG_IOMMUFD
> if (vbasedev->iommufd) {
> ops = &vfio_iommufd_ops;
> }
> #endif
>+
>+assert(ops);
>+
> return ops->attach_device(name, vbasedev, as, errp);
> }
>
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>f4a0434a5239bfb6a17b91c8879cb98e686afccc..fdf4e116570013732d4807
>1a5122d25b02da715c 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -369,10 +369,30 @@ static int vfio_get_iommu_type(VFIOContainer
>*container,
> return -EINVAL;
> }
>
>+/*
>+ * vfio_get_iommu_ops - get a VFIOIOMMUClass associated with a type
>+ */
>+static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type,
>Error **errp)
>+{
>+ObjectClass *klass = NULL;

No need to nullify?
>+
>+switch (iommu_type) {
>+case VFIO_TYPE1v2_IOMMU:
>+case VFIO_TYPE1_IOMMU:
>+klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
>+break;
>+default:
>+g_assert_not_reached();
>+};
>+
>+return VFIO_IOMMU_CLASS(klass);
>+}
>+
> static int vfio_init_container(VFIOContainer *container, int group_fd,
>VFIOAddressSpace *space, Error **errp)
> {
> int iommu_type, ret;
>+const VFIOIOMMUClass *vioc = NULL;

No need to nullify?

>
> iommu_type = vfio_get_iommu_type(container, errp);
> if (iommu_type < 0) {
>@@ -401,7 +421,14 @@ static int vfio_init_container(VFIOContainer
>*container, int group_fd,
> }
>
> container->iommu_type = iommu_type;
>-vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
>+
>+vioc = vfio_get_iommu_class(iommu_type, errp);
>+if (!vioc) {
>+error_setg(errp, "No available IOMMU models");
>+return -EINVAL;
>+}
>+
>+vfio_container_init(&container->bcontainer, space, vioc);
> return 0;
> }
>
>@@ -1098,12 +1125,26 @@ out_single:
> return ret;
> }
>
>-const VFIOIOMMUOps vfio_legacy_ops = {
>-.dma_map = vfio_legacy_dma_map,
>-.dma_unmap = vfio_legacy_dma_unmap,
>-.attach_device = vfio_legacy_attach_device,
>-.detach_device = vfio_legacy_detach_device,
>-.set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
>-.query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
>-.pci_hot_reset = vfio_legacy_pci_hot_reset,
>+static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
>+{
>+VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>+
>+vioc->dma_map = vfio_leg

RE: [PATCH for-9.0 06/10] vfio/container: Intoduce a new VFIOIOMMUClass::setup handler

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 06/10] vfio/container: Intoduce a new
>VFIOIOMMUClass::setup handler
>
>This will help in converting the sPAPR IOMMU backend to a QOM interface.
>
>Signed-off-by: Cédric Le Goater 

Reviewed-by: Zhenzhong Duan 

Thanks
Zhenzhong

>---
> include/hw/vfio/vfio-container-base.h | 1 +
> hw/vfio/container.c   | 1 +
> 2 files changed, 2 insertions(+)
>
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>a31fd9c2e3b9a571083ea8987ac27e91b332c170..870e7dc48e542ddbfc52e
>12b0ab5fab4771a1ebd 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -104,6 +104,7 @@ struct VFIOIOMMUClass {
> InterfaceClass parent_class;
>
> /* basic feature */
>+int (*setup)(VFIOContainerBase *bcontainer, Error **errp);
> int (*dma_map)(const VFIOContainerBase *bcontainer,
>hwaddr iova, ram_addr_t size,
>void *vaddr, bool readonly);
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>fdf4e116570013732d48071a5122d25b02da715c..5f5ad8479f083db0be520
>7f179f3056ae67c49c3 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -1129,6 +1129,7 @@ static void
>vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
> {
> VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>
>+vioc->setup = vfio_legacy_setup;
> vioc->dma_map = vfio_legacy_dma_map;
> vioc->dma_unmap = vfio_legacy_dma_unmap;
> vioc->attach_device = vfio_legacy_attach_device;
>--
>2.43.0



Re: [PATCH V7 02/12] cpus: stop vm in suspended runstate

2023-12-10 Thread Peter Xu
On Wed, Dec 06, 2023 at 10:09:32PM +0100, Philippe Mathieu-Daudé wrote:
> What about runstate_is_vcpu_clock_ticking() then?

The problem is vCPU clock ticks can only be one part of "VM is running"
state (no matter whether vCPUs are running or not).  I'm even thinking
whether cpu_enable_ticks() should one day be put into a vm state change
notifier instead to be even clearer that it's not special (I hope I didn't
overlook its specialty..).

Thanks,

-- 
Peter Xu




RE: [PATCH for-9.0 07/10] vfio/spapr: Introduce a sPAPR VFIOIOMMU QOM interface

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 07/10] vfio/spapr: Introduce a sPAPR VFIOIOMMU
>QOM interface
>
>Move vfio_spapr_container_setup() to a VFIOIOMMUClass::setup handler
>and convert the sPAPR VFIOIOMMUOps struct to a QOM interface. The
>sPAPR QOM interface inherits from the legacy QOM interface because
>because both have the same basic needs. The sPAPR interface is then
>extended with the handlers specific to the sPAPR IOMMU.
>
>This allows reuse and provides better abstraction of the backends. It
>will be useful to avoid compiling the sPAPR IOMMU backend on targets
>not supporting it.
>
>Signed-off-by: Cédric Le Goater 
>---
> include/hw/vfio/vfio-container-base.h |  1 +
> hw/vfio/container.c   | 18 
> hw/vfio/spapr.c   | 40 +--
> 3 files changed, 32 insertions(+), 27 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>870e7dc48e542ddbfc52e12b0ab5fab4771a1ebd..4012360c07b7c0a23f170f
>94a19455c79d3504b1 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -96,6 +96,7 @@ typedef struct VFIOIOMMU VFIOIOMMU;
>
> #define TYPE_VFIO_IOMMU "vfio-iommu"
> #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>+#define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
>
> #define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj),
>TYPE_VFIO_IOMMU)
> DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU,
>TYPE_VFIO_IOMMU)
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>5f5ad8479f083db0be5207f179f3056ae67c49c3..ce5a731ba74600fbb331a8
>0f5148a88e2e43b068 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -381,6 +381,10 @@ static const VFIOIOMMUClass
>*vfio_get_iommu_class(int iommu_type, Error **errp)
> case VFIO_TYPE1_IOMMU:
> klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
> break;
>+case VFIO_SPAPR_TCE_v2_IOMMU:
>+case VFIO_SPAPR_TCE_IOMMU:
>+klass = object_class_by_name(TYPE_VFIO_IOMMU_SPAPR);
>+break;
> default:
> g_assert_not_reached();
> };
>@@ -623,19 +627,9 @@ static int vfio_connect_container(VFIOGroup
>*group, AddressSpace *as,
> goto free_container_exit;
> }
>
>-switch (container->iommu_type) {
>-case VFIO_TYPE1v2_IOMMU:
>-case VFIO_TYPE1_IOMMU:
>-ret = vfio_legacy_setup(bcontainer, errp);
>-break;
>-case VFIO_SPAPR_TCE_v2_IOMMU:
>-case VFIO_SPAPR_TCE_IOMMU:
>-ret = vfio_spapr_container_init(container, errp);
>-break;
>-default:
>-g_assert_not_reached();
>-}
>+assert(bcontainer->ops->setup);
>
>+ret = bcontainer->ops->setup(bcontainer, errp);
> if (ret) {
> goto enable_discards_exit;
> }
>diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
>index
>44617dfc6b5f1a2a3a1c37436b76042aebda8b63..46aa14bd2ae6d580c16bb
>a75838cb6aca7d4047f 100644
>--- a/hw/vfio/spapr.c
>+++ b/hw/vfio/spapr.c
>@@ -458,20 +458,11 @@ static void
>vfio_spapr_container_release(VFIOContainerBase *bcontainer)
> }
> }
>
>-static VFIOIOMMUOps vfio_iommu_spapr_ops;
>-
>-static void setup_spapr_ops(VFIOContainerBase *bcontainer)
>-{
>-vfio_iommu_spapr_ops = *bcontainer->ops;
>-vfio_iommu_spapr_ops.add_window =
>vfio_spapr_container_add_section_window;
>-vfio_iommu_spapr_ops.del_window =
>vfio_spapr_container_del_section_window;
>-vfio_iommu_spapr_ops.release = vfio_spapr_container_release;
>-bcontainer->ops = &vfio_iommu_spapr_ops;
>-}
>-
>-int vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>+static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
>+  Error **errp)
> {
>-VFIOContainerBase *bcontainer = &container->bcontainer;
>+VFIOContainer *container = container_of(bcontainer, VFIOContainer,
>+bcontainer);
> VFIOSpaprContainer *scontainer = container_of(container,
>VFIOSpaprContainer,
>   container);
> struct vfio_iommu_spapr_tce_info info;
>@@ -536,8 +527,6 @@ int vfio_spapr_container_init(VFIOContainer
>*container, Error **errp)
>   0x1000);
> }
>
>-setup_spapr_ops(bcontainer);
>-
> return 0;
>
> listener_unregister_exit:
>@@ -546,3 +535,24 @@ listener_unregister_exit:
> }
> return ret;
> }
>+
>+static void vfio_iommu_spapr_class_init(ObjectClass *klass, void *data)
>+{
>+VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>+
>+vioc->add_window = vfio_spapr_container_add_section_window;
>+vioc->del_window = vfio_spapr_container_del_section_window;
>+vioc->release = vfio_spapr_container_release;
>+vioc->setup = vfio_spapr_container_setup;
>+};
>+
>+static const TypeInfo types[] = {
>+{
>+.name = TYPE_VFIO_IOMMU_SPAPR,
>+ 

RE: [PATCH for-9.0 08/10] vfio/iommufd: Introduce a VFIOIOMMU iommufd QOM interface

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 08/10] vfio/iommufd: Introduce a VFIOIOMMU
>iommufd QOM interface
>
>As previously done for the sPAPR and legacy IOMMU backends, convert
>the VFIOIOMMUOps struct to a QOM interface. The set of of operations
>for this backend can be referenced with a literal typename instead of
>a C struct.
>
>Signed-off-by: Cédric Le Goater 

Reviewed-by: Zhenzhong Duan 

Thanks
Zhenzhong

>---
> include/hw/vfio/vfio-common.h |  1 -
> include/hw/vfio/vfio-container-base.h |  2 +-
> hw/vfio/common.c  |  2 +-
> hw/vfio/iommufd.c | 36 ---
> 4 files changed, 29 insertions(+), 12 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>index
>14c497b6b0a79466e8f567aceed384ec2c75ea90..9b7ef7d02b5a0ad5266bcc
>4d06cd6874178978e4 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup)
>VFIOGroupList;
> typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
> extern VFIOGroupList vfio_group_list;
> extern VFIODeviceList vfio_device_list;
>-extern const VFIOIOMMUOps vfio_iommufd_ops;
> extern const MemoryListener vfio_memory_listener;
> extern int vfio_kvm_device_fd;
>
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>4012360c07b7c0a23f170f94a19455c79d3504b1..5fd02fab5fd627dc601068
>5e9d956ba20ee329fd 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -17,7 +17,6 @@
>
> typedef struct VFIODevice VFIODevice;
> typedef struct VFIOIOMMUClass VFIOIOMMUClass;
>-#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
>
> typedef struct {
> unsigned long *bitmap;
>@@ -97,6 +96,7 @@ typedef struct VFIOIOMMU VFIOIOMMU;
> #define TYPE_VFIO_IOMMU "vfio-iommu"
> #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
> #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
>+#define TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
>
> #define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj),
>TYPE_VFIO_IOMMU)
> DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU,
>TYPE_VFIO_IOMMU)
>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>index
>2329d0efc8c1d617f0bfee5283e82b295d2d477d..89ff1c7aeda14d20b2e24f
>8bc251db0a71d4527c 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1508,7 +1508,7 @@ int vfio_attach_device(char *name, VFIODevice
>*vbasedev,
>
> #ifdef CONFIG_IOMMUFD
> if (vbasedev->iommufd) {
>-ops = &vfio_iommufd_ops;
>+ops =
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
>D));
> }
> #endif
>
>diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>index
>87a561c54580adc6d7b2711331a00940ff13bd43..807be2545eb147a6fac97
>3752cf37eeede1a4ff6 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -319,6 +319,8 @@ static int iommufd_cdev_attach(const char *name,
>VFIODevice *vbasedev,
> int ret, devfd;
> uint32_t ioas_id;
> Error *err = NULL;
>+const VFIOIOMMUClass *iommufd_vioc =
>+
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
>D));
>
> if (vbasedev->fd < 0) {
> devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
>@@ -340,7 +342,7 @@ static int iommufd_cdev_attach(const char *name,
>VFIODevice *vbasedev,
> /* try to attach to an existing container in this space */
> QLIST_FOREACH(bcontainer, &space->containers, next) {
> container = container_of(bcontainer, VFIOIOMMUFDContainer,
>bcontainer);
>-if (bcontainer->ops != &vfio_iommufd_ops ||
>+if (bcontainer->ops != iommufd_vioc ||
> vbasedev->iommufd != container->be) {
> continue;
> }
>@@ -374,7 +376,7 @@ static int iommufd_cdev_attach(const char *name,
>VFIODevice *vbasedev,
> container->ioas_id = ioas_id;
>
> bcontainer = &container->bcontainer;
>-vfio_container_init(bcontainer, space, &vfio_iommufd_ops);
>+vfio_container_init(bcontainer, space, iommufd_vioc);
> QLIST_INSERT_HEAD(&space->containers, bcontainer, next);
>
> ret = iommufd_cdev_attach_container(vbasedev, container, errp);
>@@ -476,9 +478,11 @@ static void iommufd_cdev_detach(VFIODevice
>*vbasedev)
> static VFIODevice *iommufd_cdev_pci_find_by_devid(__u32 devid)
> {
> VFIODevice *vbasedev_iter;
>+const VFIOIOMMUClass *iommufd_vioc =
>+
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
>D));
>
> QLIST_FOREACH(vbasedev_iter, &vfio_device_list, global_next) {
>-if (vbasedev_iter->bcontainer->ops != &vfio_iommufd_ops) {
>+if (vbasedev_iter->bcontainer->ops != iommufd_vioc) {
> continue;
> }
> if (devid == vbasedev_iter->devid) {
>@@ -621,10 +625,24 @@ out_single:
> return ret;
> }
>
>-const VFIOIOMMUOps vfio_iommufd_ops = {
>-.dma_map = iommufd_cdev_map,
>-

RE: [PATCH for-9.0 09/10] vfio/spapr: Only compile sPAPR IOMMU support when needed

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 09/10] vfio/spapr: Only compile sPAPR IOMMU
>support when needed
>
>sPAPR IOMMU support is only needed for pseries machines. Compile out
>support when CONFIG_PSERIES is not set. This saves ~7K of text.
>
>Signed-off-by: Cédric Le Goater 

Reviewed-by: Zhenzhong Duan 

Thanks
Zhenzhong

>---
> hw/vfio/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
>index
>e5d98b6adc223061f6b0c3e1a7db3ba93d4eef16..bb98493b53e858c53181e
>224f9cb46892838a8be 100644
>--- a/hw/vfio/meson.build
>+++ b/hw/vfio/meson.build
>@@ -4,9 +4,9 @@ vfio_ss.add(files(
>   'common.c',
>   'container-base.c',
>   'container.c',
>-  'spapr.c',
>   'migration.c',
> ))
>+vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
> vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
>   'iommufd.c',
> ))
>--
>2.43.0



RE: [PATCH for-9.0 10/10] vfio/iommufd: Remove CONFIG_IOMMUFD usage

2023-12-10 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Subject: [PATCH for-9.0 10/10] vfio/iommufd: Remove CONFIG_IOMMUFD
>usage
>
>Availability of the IOMMUFD backend can now be fully determined at
>runtime and the ifdef check was a build time protection (for PPC not
>supporting it mostly).
>
>Signed-off-by: Cédric Le Goater 

Reviewed-by: Zhenzhong Duan 

Thanks
Zhenzhong



Re: [PATCH V7 05/12] migration: propagate suspended runstate

2023-12-10 Thread Peter Xu
On Wed, Dec 06, 2023 at 09:23:30AM -0800, Steve Sistare wrote:
> If the outgoing machine was previously suspended, propagate that to the
> incoming side via global_state, so a subsequent vm_start restores the
> suspended state.  To maintain backward and forward compatibility, reclaim
> some space from the runstate member.
> 
> Signed-off-by: Steve Sistare 

Reviewed-by: Peter Xu 

One nitpick below.

> ---
>  migration/global_state.c | 35 +--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 4e2a9d8..d4f61a1 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -22,7 +22,16 @@
>  
>  typedef struct {
>  uint32_t size;
> -uint8_t runstate[100];
> +
> +/*
> + * runstate was 100 bytes, zero padded, but we trimmed it to add a
> + * few fields and maintain backwards compatibility.
> + */
> +uint8_t runstate[32];
> +uint8_t has_vm_was_suspended;
> +uint8_t vm_was_suspended;
> +uint8_t unused[66];
> +
>  RunState state;
>  bool received;
>  } GlobalState;
> @@ -35,6 +44,10 @@ static void global_state_do_store(RunState state)
>  assert(strlen(state_str) < sizeof(global_state.runstate));
>  strpadcpy((char *)global_state.runstate, sizeof(global_state.runstate),
>state_str, '\0');
> +global_state.has_vm_was_suspended = true;
> +global_state.vm_was_suspended = vm_get_suspended();
> +
> +memset(global_state.unused, 0, sizeof(global_state.unused));
>  }
>  
>  void global_state_store(void)
> @@ -68,6 +81,12 @@ static bool global_state_needed(void *opaque)
>  return true;
>  }
>  
> +/* If the suspended state must be remembered, it is needed */
> +
> +if (vm_get_suspended()) {
> +return true;
> +}

Can we drop this section?

I felt unsafe when QEMU can overwrite the option even if user explicitly
specified store-global-state=off but we still send this..  Ideally I think
it's better if it's as simple as:

static bool global_state_needed(void *opaque)
{
return migrate_get_current()->store_global_state;
}

I don't think we can remove the old trick due to compatibility reasons, but
maybe nice to not add new ones to make this section more unpredictable in
the migration stream?

IMHO it shouldn't matter in reality for the current use case even dropping
it, as I don't expect any non-Xen QEMU VMs migrates without having the
option turned on (store-global-state=on) after QEMU 2.4.

Thanks,

-- 
Peter Xu




Re: [PATCH v2] qdev: Report an error for machine without HotplugHandler

2023-12-10 Thread Markus Armbruster
Akihiko Odaki  writes:

> The HotplugHandler of the machine will be used when the parent bus does
> not exist, but the machine may not have one. Report an error in such a
> case instead of aborting.
>
> Fixes: 7716b8ca74 ("qdev: HotplugHandler: Add support for unplugging BUS-less 
> devices")
> Signed-off-by: Akihiko Odaki 

Do you have a reproducer for the crash?

> ---
> Changes in v2:
> - Fixed indention.
> - Link to v1: 
> https://lore.kernel.org/r/20231202-bus-v1-1-f7540e3a8...@daynix.com
> ---
>  system/qdev-monitor.c | 13 ++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index a13db763e5..5fe5d49c20 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -927,9 +927,16 @@ void qdev_unplug(DeviceState *dev, Error **errp)
   void qdev_unplug(DeviceState *dev, Error **errp)
   {
   DeviceClass *dc = DEVICE_GET_CLASS(dev);
   HotplugHandler *hotplug_ctrl;
   HotplugHandlerClass *hdc;
   Error *local_err = NULL;

   if (qdev_unplug_blocked(dev, errp)) {
   return;
   }

   if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
   error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
   return;
   }

   if (!dc->hotpluggable) {
   error_setg(errp, QERR_DEVICE_NO_HOTPLUG,
  object_get_typename(OBJECT(dev)));
   return;
   }

   if (!migration_is_idle() && !dev->allow_unplug_during_migration) {
   error_setg(errp, "device_del not allowed while migrating");
   return;
   }

>  qdev_hot_removed = true;
>  
>  hotplug_ctrl = qdev_get_hotplug_handler(dev);
> -/* hotpluggable device MUST have HotplugHandler, if it doesn't
> - * then something is very wrong with it */
> -g_assert(hotplug_ctrl);
> +if (!hotplug_ctrl) {
> +/*
> + * hotpluggable bus MUST have HotplugHandler, if it doesn't
> + * then something is very wrong with it
> + */
> +assert(!dev->parent_bus);
> +
> +error_setg(errp, "The machine does not support hotplugging for a 
> device without parent bus");
> +return;
> +}

Extended version of my question above: what are the devices where
qdev_get_hotplug_handler(dev) returns null here?

>  
>  /* If device supports async unplug just request it to be done,
>   * otherwise just remove it synchronously */
>
> ---
> base-commit: 4705fc0c8511d073bee4751c3c974aab2b10a970
> change-id: 20231202-bus-75a454c5d959
>
> Best regards,




Re: [PATCH V7 11/12] tests/qtest: precopy migration with suspend

2023-12-10 Thread Peter Xu
On Wed, Dec 06, 2023 at 09:23:36AM -0800, Steve Sistare wrote:
> Add a test case to verify that the suspended state is handled correctly
> during live migration precopy.  The test suspends the src, migrates, then
> wakes the dest.
> 
> Signed-off-by: Steve Sistare 

Reviewed-by: Peter Xu 

-- 
Peter Xu




Re: [PATCH V7 12/12] tests/qtest: postcopy migration with suspend

2023-12-10 Thread Peter Xu
On Wed, Dec 06, 2023 at 09:23:37AM -0800, Steve Sistare wrote:
> Add a test case to verify that the suspended state is handled correctly by
> live migration postcopy.  The test suspends the src, migrates, then wakes
> the dest.
> 
> Signed-off-by: Steve Sistare 

Reviewed-by: Peter Xu 

-- 
Peter Xu




Re: [PATCH V7 00/12] fix migration of suspended runstate

2023-12-10 Thread Peter Xu
On Wed, Dec 06, 2023 at 12:30:02PM -0500, Steven Sistare wrote:
> cpus: stop vm in suspended runstate

This patch still didn't copy the QAPI maintainers, please remember to do so
in a new post.

Maybe it would be easier to move the QAPI doc changes into a separate
patch?

Thanks,

-- 
Peter Xu




[PATCH 0/2] support unaligned access for some xHCI registers

2023-12-10 Thread Tomoyuki HIROSE
According to xHCI spec rev 1.2, unaligned access to xHCI Host
Controller Capability Registers are not prohibited. But current
implementation does not support unaligned access to 'MemoryRegion'.
These patches contain 2 changes:
1. support unaligned access to 'MemoryRegion'
2. allow unaligned access to Host Controller Capability Registers.

Tomoyuki HIROSE (2):
  system/memory.c: support unaligned access
  hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

 hw/usb/hcd-xhci.c |  4 +++-
 system/memory.c   | 22 --
 2 files changed, 19 insertions(+), 7 deletions(-)

-- 
2.39.2




[PATCH 1/2] system/memory.c: support unaligned access

2023-12-10 Thread Tomoyuki HIROSE
The previous code ignored 'impl.unaligned' and handled unaligned accesses
as is. But this implementation cannot emulate specific registers of some
devices that allow unaligned access such as xHCI Host Controller Capability
Registers.
This commit checks 'impl.unaligned' and if it is false, QEMU emulates
unaligned access with multiple aligned access.

Signed-off-by: Tomoyuki HIROSE 
---
 system/memory.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 798b6c0a17..b0caa90fef 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -539,6 +539,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
 unsigned i;
 MemTxResult r = MEMTX_OK;
 bool reentrancy_guard_applied = false;
+hwaddr aligned_addr;
+unsigned corrected_size = size;
+signed align_diff = 0;
 
 if (!access_size_min) {
 access_size_min = 1;
@@ -560,18 +563,25 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
 reentrancy_guard_applied = true;
 }
 
-/* FIXME: support unaligned access? */
 access_size = MAX(MIN(size, access_size_max), access_size_min);
 access_mask = MAKE_64BIT_MASK(0, access_size * 8);
+if (!mr->ops->impl.unaligned) {
+aligned_addr = addr & ~(access_size - 1);
+align_diff = addr - aligned_addr;
+corrected_size = size < access_size ? access_size :
+size + (align_diff > 0 ? access_size : 0);
+addr = aligned_addr;
+}
 if (memory_region_big_endian(mr)) {
-for (i = 0; i < size; i += access_size) {
+for (i = 0; i < corrected_size; i += access_size) {
 r |= access_fn(mr, addr + i, value, access_size,
-(size - access_size - i) * 8, access_mask, attrs);
+(size - access_size - i + align_diff) * 8,
+access_mask, attrs);
 }
 } else {
-for (i = 0; i < size; i += access_size) {
-r |= access_fn(mr, addr + i, value, access_size, i * 8,
-access_mask, attrs);
+for (i = 0; i < corrected_size; i += access_size) {
+r |= access_fn(mr, addr + i, value, access_size,
+((signed)i - align_diff) * 8, access_mask, attrs);
 }
 }
 if (mr->dev && reentrancy_guard_applied) {
-- 
2.39.2




[PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

2023-12-10 Thread Tomoyuki HIROSE
According to xHCI spec rev 1.2, unaligned access to xHCI Host
Controller Capability Registers is not prohibited. In Addition, the
limit of access size is also unspecified. Actually, some real devices
allow unaligned access and 8-byte access to these registers.
This commit makes it possible to unaligned access and 8-byte access
to Host Controller Capability Registers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/143
Signed-off-by: Tomoyuki HIROSE 
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4b60114207..41abeb9ac5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3181,9 +3181,11 @@ static const MemoryRegionOps xhci_cap_ops = {
 .read = xhci_cap_read,
 .write = xhci_cap_write,
 .valid.min_access_size = 1,
-.valid.max_access_size = 4,
+.valid.max_access_size = 8,
+.valid.unaligned = true,
 .impl.min_access_size = 4,
 .impl.max_access_size = 4,
+.impl.unaligned = false,
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-- 
2.39.2




Re: [PATCH RFC v2 00/12] virtio-net: add support for SR-IOV emulation

2023-12-10 Thread Jason Wang
On Mon, Dec 11, 2023 at 1:30 PM Akihiko Odaki  wrote:
>
> On 2023/12/11 11:52, Jason Wang wrote:
> > On Sun, Dec 10, 2023 at 12:06 PM Akihiko Odaki  
> > wrote:
> >>
> >> Introduction
> >> 
> >>
> >> This series is based on the RFC series submitted by Yui Washizu[1].
> >> See also [2] for the context.
> >>
> >> This series enables SR-IOV emulation for virtio-net. It is useful
> >> to test SR-IOV support on the guest, or to expose several vDPA devices
> >> in a VM. vDPA devices can also provide L2 switching feature for
> >> offloading though it is out of scope to allow the guest to configure
> >> such a feature.
> >>
> >> The PF side code resides in virtio-pci. The VF side code resides in
> >> the PCI common infrastructure, but it is restricted to work only for
> >> virtio-net-pci because of lack of validation.
> >>
> >> User Interface
> >> --
> >>
> >> A user can configure a SR-IOV capable virtio-net device by adding
> >> virtio-net-pci functions to a bus. Below is a command line example:
> >>-netdev user,id=n -netdev user,id=o
> >>-netdev user,id=p -netdev user,id=q
> >>-device pcie-root-port,id=b
> >>-device virtio-net-pci,bus=b,addr=0x0.0x3,netdev=q,sriov-pf=f
> >>-device virtio-net-pci,bus=b,addr=0x0.0x2,netdev=p,sriov-pf=f
> >>-device virtio-net-pci,bus=b,addr=0x0.0x1,netdev=o,sriov-pf=f
> >>-device virtio-net-pci,bus=b,addr=0x0.0x0,netdev=n,id=f
> >>
> >> The VFs specify the paired PF with "sriov-pf" property. The PF must be
> >> added after all VFs. It is user's responsibility to ensure that VFs have
> >> function numbers larger than one of the PF, and the function numbers
> >> have a consistent stride.
> >
> > This seems not user friendly. Any reason we can't just allow user to
> > specify the stride here?
>
> It should be possible to assign addr automatically without requiring
> user to specify the stride. I'll try that in the next version.
>
> >
> > Btw, I vaguely remember qemu allows the params to be accepted as a
> > list. If this is true, we can accept a list of netdev here?
>
> Yes, rocker does that. But the problem is not just about getting
> parameters needed for VFs, which I forgot to mention in the cover letter
> and will explain below.
>
> >
> >>
> >> Keeping VF instances
> >> 
> >>
> >> A problem with SR-IOV emulation is that it needs to hotplug the VFs as
> >> the guest requests. Previously, this behavior was implemented by
> >> realizing and unrealizing VFs at runtime. However, this strategy does
> >> not work well for the proposed virtio-net emulation; in this proposal,
> >> device options passed in the command line must be maintained as VFs
> >> are hotplugged, but they are consumed when the machine starts and not
> >> available after that, which makes realizing VFs at runtime impossible.
> >
> > Could we store the device options in the PF?
>
> I wrote it's to store the device options, but the problem is actually
> more about realizing VFs at runtime instead of at the initialization time.
>
> Realizing VFs at runtime have two major problems. One is that it delays
> the validations of options; invalid options will be noticed when the
> guest requests to realize VFs.

If PCI spec allows the failure when creating VF, then it should not be
a problem.

> netdevs also warn that they are not used
> at initialization time, not knowing that they will be used by VFs later.

We could invent things to calm down this false positive.

> References to other QEMU objects in the option may also die before VFs
> are realized.

Is there any other thing than netdev we need to consider?

>
> The other problem is that QEMU cannot interact with the unrealized VFs.
> For example, if you type "device_add virtio-net-pci,id=vf,sriov-pf=pf"
> in HMP, you will expect "device_del vf" works, but it's hard to
> implement such behaviors with unrealized VFs.

I think hotplug can only be done at PF level if we do that.

>
> I was first going to compromise and allow such quirky behaviors, but I
> realized such a compromise is unnecessary if we reuse the PCI power down
> logic so I wrote v2.

Haven't checked the code, but anything related to the PM here?

Thanks

>
> Regards,
> Akihiko Odaki
>




Re: [PATCH 01/40] linux-headers: add vhost_types.h and vhost.h

2023-12-10 Thread Eugenio Perez Martin
On Thu, Dec 7, 2023 at 7:50 PM Si-Wei Liu  wrote:
>
> Signed-off-by: Si-Wei Liu 

This should be updated from scripts/update-linux-headers.sh.

> ---
>  include/standard-headers/linux/vhost_types.h | 13 +
>  linux-headers/linux/vhost.h  |  9 +
>  2 files changed, 22 insertions(+)
>
> diff --git a/include/standard-headers/linux/vhost_types.h 
> b/include/standard-headers/linux/vhost_types.h
> index 5ad07e1..c39199b 100644
> --- a/include/standard-headers/linux/vhost_types.h
> +++ b/include/standard-headers/linux/vhost_types.h
> @@ -185,5 +185,18 @@ struct vhost_vdpa_iova_range {
>   * DRIVER_OK
>   */
>  #define VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK  0x6
> +/* Device can be resumed */
> +#define VHOST_BACKEND_F_RESUME  0x5
> +/* Device supports the driver enabling virtqueues both before and after
> + * DRIVER_OK
> + */
> +#define VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK  0x6
> +/* Device may expose the virtqueue's descriptor area, driver area and
> + * device area to a different group for ASID binding than where its
> + * buffers may reside. Requires VHOST_BACKEND_F_IOTLB_ASID.
> + */
> +#define VHOST_BACKEND_F_DESC_ASID0x7
> +/* IOTLB don't flush memory mapping across device reset */
> +#define VHOST_BACKEND_F_IOTLB_PERSIST  0x8
>
>  #endif
> diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
> index f5c48b6..c61c687 100644
> --- a/linux-headers/linux/vhost.h
> +++ b/linux-headers/linux/vhost.h
> @@ -219,4 +219,13 @@
>   */
>  #define VHOST_VDPA_RESUME  _IO(VHOST_VIRTIO, 0x7E)
>
> +/* Get the dedicated group for the descriptor table of a virtqueue:
> + * read index, write group in num.
> + * The virtqueue index is stored in the index field of vhost_vring_state.
> + * The group id for the descriptor table of this specific virtqueue
> + * is returned via num field of vhost_vring_state.
> + */
> +#define VHOST_VDPA_GET_VRING_DESC_GROUP_IOWR(VHOST_VIRTIO, 0x7F, 
>   \
> + struct vhost_vring_state)
> +
>  #endif
> --
> 1.8.3.1
>