Re: [PATCH v2 1/3] qtest: migration: Enhance qtest migration functions to support 'channels' argument
On 24/02/24 6:18 pm, Het Gala wrote: On 24/02/24 1:42 am, Fabiano Rosas wrote: Het Gala writes: Introduce support for adding a 'channels' argument to migrate_qmp_fail, migrate_incoming_qmp and migrate_qmp functions within the migration qtest framework, enabling enhanced control over migration scenarios. [...] Write the test like this: static void test_multifd_tcp_none_channels(void) { MigrateCommon args = { .listen_uri = "defer", .start_hook = test_migrate_precopy_tcp_multifd_start, .live = true, .connect_channels = "'channels': [ { 'channel-type': 'main'," " 'addr': { 'transport': 'socket'," "'type': 'inet'," "'host': '127.0.0.1'," "'port': '0' } } ]", .connect_uri = NULL; }; test_precopy_common(&args); } this was the same first approach that I attempted. It won't work because The final 'migrate' QAPI with channels string would look like { "execute": "migrate", "arguments": { "channels": "[ { "channel-type": "main", "addr": { "transport": "socket", "type": "inet", "host": "10.117.29.84", "port": "4000" }, "multifd-channels": 2 } ]" } } Sorry, In your example given above, the output looks like : {"execute": "migrate", "arguments": {"uri": "tcp:127.0.0.1:0", "channels": "'channels': [ { 'channel-type': 'main', 'addr': { 'transport': 'socket', 'type': 'inet', 'host': '127.0.0.1', 'port': '0' } } ]"}} instead of { "execute": "migrate", "arguments": { "channels": [ { "channel-type": "main", "addr": { "transport": "socket", "type": "inet", "host": "10.117.29.84", "port": "4000" }, "multifd-channels": 2 } ] } } It would complain, that channels should be an *array* and not a string. because of /"channels": "'channels': [ { 'channel-type': / [...] Regards, Het Gala Regards, Het Gala
Re: [PULL v2 00/39] tcg and linux-user patch queue
On Sat, 24 Feb 2024 at 01:06, Richard Henderson wrote: > > v2: Fix bsd-user build errors. > > > r~ > > > The following changes since commit 3d54cbf269d63ff1d500b35b2bcf4565ff8ad485: > > Merge tag 'hw-misc-20240222' of https://github.com/philmd/qemu into staging > (2024-02-22 15:44:29 +) > > are available in the Git repository at: > > https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20240222-2 > > for you to fetch changes up to fcc6ad372f56d3f47b6d5457a904916b48b9e114: > > linux-user: Remove pgb_dynamic alignment assertion (2024-02-23 15:07:03 > -0800) > > > tcg/aarch64: Apple does not align __int128_t in even registers > accel/tcg: Fixes for page tables in mmio memory > linux-user: Remove qemu_host_page_{size,mask}, HOST_PAGE_ALIGN > migration: Remove qemu_host_page_size > hw/tpm: Remove qemu_host_page_size > softmmu: Remove qemu_host_page_{size,mask}, HOST_PAGE_ALIGN > linux-user: Split and reorganize target_mmap. > *-user: Deprecate and disable -p pagesize > linux-user: Allow TARGET_PAGE_BITS_VARY > target/alpha: Enable TARGET_PAGE_BITS_VARY for user-only > target/arm: Enable TARGET_PAGE_BITS_VARY for AArch64 user-only > target/ppc: Enable TARGET_PAGE_BITS_VARY for user-only > linux-user: Remove pgb_dynamic alignment assertion > > Hi -- looks like this introduces an new variable-length-array, which we are trying to get rid of: ../linux-user/elfload.c: In function 'vma_dump_size': ../linux-user/elfload.c:4254:9: error: ISO C90 forbids variable length array 'page' [-Werror=vla] 4254 | char page[TARGET_PAGE_SIZE]; | ^~~~ ../linux-user/elfload.c: In function 'elf_core_dump': ../linux-user/elfload.c:4778:13: error: ISO C90 forbids variable length array 'page' [-Werror=vla] 4778 | char page[TARGET_PAGE_SIZE]; | ^~~~ I noticed this because I happened to test merging this pullreq together with Thomas's testing pullreq that enforces the -Wvla error. I'll be merging that testing pull shortly but it's not upstream quite yet. thanks -- PMM
Re: [PATCH] scripts/qemu-binfmt-conf.sh: refresh
23.02.2024 18:55, Peter Maydell: On Sun, 18 Feb 2024 at 09:32, Michael Tokarev wrote: 09.09.2023 16:23, Michael Tokarev : A friendly ping? A friendly ping #2? Looking at the patch, the commit message lists 9 separate things it does. That suggests it ought to be a 9-patch patchset, not a single patch. I bet most of those 9 would be much easier to review than this, too... When I was doing that, I tried to split things up. But it's.. difficult. Having in mind the amount of issues this script has. Trying to make small changes would be a huge work. All the mentioned points (well, most) are the effect of an almost rewrite. I tried to switch to this script in debian (from the debian-specific thing which was used as a prototype for qemu-binfmt-conf.sh) but failed, that was the result of a minimal cleanup.. And yes, I know it's difficult to review. There's a trade-off - either make it good, or make it easier to review ;) Dunno what to do with it now :) Thanks, /mjt
[Bug 2054889] [NEW] pcap streams are text files which insert 0xD in Windows version
Public bug reported: Since Windows text files use CRLFs for all \n, the Windows version of QEMU inserts a CR in the PCAP stream when a LF is encountered when using USB PCAP files. Starting at line 275 in hw/usb/bus (https://gitlab.com/qemu- project/qemu/-/blob/master/hw/usb/bus.c?ref_type=heads#L275), the file is opened as text instead of binary. I think the following patch would fix the issue: if (dev->pcap_filename) { - int fd = qemu_open_old(dev->pcap_filename, O_CREAT | O_WRONLY | O_TRUNC, 0666); + int fd = qemu_open_old(dev->pcap_filename, O_CREAT | O_WRONLY | O_TRUNC | O_BINARY, 0666); if (fd < 0) { error_setg(errp, "open %s failed", dev->pcap_filename); usb_qdev_unrealize(qdev); return; } - dev->pcap = fdopen(fd, "w"); + dev->pcap = fdopen(fd, "wb"); usb_pcap_init(dev->pcap); } To show an example, when using a very common protocol to USB disks, the BBB protocol uses a 10-byte command packet. For example, the READ_CAPACITY(10) command (implemented at https://gitlab.com/qemu- project/qemu/-/blob/master/hw/scsi/scsi-disk.c#L2068) will have a command block length of 10 (0xA). When this 10-byte command (part of the 31-byte CBW) is placed into the PCAP file, the Windows file manager inserts a 0xD before the 0xA, turning the 31-byte CBW into a 32-byte CBW. Actual CBW: 0040 55 53 42 43 01 00 00 00 08 00 00 00 80 00 0a 25 USBC 0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 %.. PCAP CBW 0040 55 53 42 43 01 00 00 00 08 00 00 00 80 00 0d 0a USBC 0050 25 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 %.. I believe simply opening the PCAP file as BINARY instead of TEXT will fix this issue. Thank you. ** Affects: qemu Importance: Undecided Status: New ** Tags: pcap usb -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/2054889 Title: pcap streams are text files which insert 0xD in Windows version Status in QEMU: New Bug description: Since Windows text files use CRLFs for all \n, the Windows version of QEMU inserts a CR in the PCAP stream when a LF is encountered when using USB PCAP files. Starting at line 275 in hw/usb/bus (https://gitlab.com/qemu- project/qemu/-/blob/master/hw/usb/bus.c?ref_type=heads#L275), the file is opened as text instead of binary. I think the following patch would fix the issue: if (dev->pcap_filename) { - int fd = qemu_open_old(dev->pcap_filename, O_CREAT | O_WRONLY | O_TRUNC, 0666); + int fd = qemu_open_old(dev->pcap_filename, O_CREAT | O_WRONLY | O_TRUNC | O_BINARY, 0666); if (fd < 0) { error_setg(errp, "open %s failed", dev->pcap_filename); usb_qdev_unrealize(qdev); return; } - dev->pcap = fdopen(fd, "w"); + dev->pcap = fdopen(fd, "wb"); usb_pcap_init(dev->pcap); } To show an example, when using a very common protocol to USB disks, the BBB protocol uses a 10-byte command packet. For example, the READ_CAPACITY(10) command (implemented at https://gitlab.com/qemu- project/qemu/-/blob/master/hw/scsi/scsi-disk.c#L2068) will have a command block length of 10 (0xA). When this 10-byte command (part of the 31-byte CBW) is placed into the PCAP file, the Windows file manager inserts a 0xD before the 0xA, turning the 31-byte CBW into a 32-byte CBW. Actual CBW: 0040 55 53 42 43 01 00 00 00 08 00 00 00 80 00 0a 25 USBC 0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 %.. PCAP CBW 0040 55 53 42 43 01 00 00 00 08 00 00 00 80 00 0d 0a USBC 0050 25 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 %.. I believe simply opening the PCAP file as BINARY instead of TEXT will fix this issue. Thank you. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/2054889/+subscriptions
Re: [PATCH] target/riscv: Fix shift count overflow
On 2/24/24 10:02, demin.han wrote: The result of (8 - 3 - vlmul) is negtive when vlmul >= 6, and results in wrong vill. Signed-off-by: demin.han --- target/riscv/vector_helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index 84cec73eb2..ced0aca633 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -53,10 +53,9 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1, * VLEN * LMUL >= SEW * VLEN >> (8 - lmul) >= sew * (vlenb << 3) >> (8 - lmul) >= sew - * vlenb >> (8 - 3 - lmul) >= sew */ if (vlmul == 4 || -cpu->cfg.vlenb >> (8 - 3 - vlmul) < sew) { +(cpu->cfg.vlenb << 3) >> (8 - vlmul) < sew) { vill = true; } Please add a new var: uint16_t vlen = cpu->cfg.vlenb << 3; And use it in the 'if' to be more readable: if (vlmul == 4 || vlen >> (8 - 3 - vlmul) < sew) { vill = true; } Thanks, Daniel }
[Bug 2054889] Re: pcap streams are text files which insert 0xD in Windows version
** Changed in: qemu Status: New => Confirmed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/2054889 Title: pcap streams are text files which insert 0xD in Windows version Status in QEMU: Confirmed Bug description: Since Windows text files use CRLFs for all \n, the Windows version of QEMU inserts a CR in the PCAP stream when a LF is encountered when using USB PCAP files. Starting at line 275 in hw/usb/bus (https://gitlab.com/qemu- project/qemu/-/blob/master/hw/usb/bus.c?ref_type=heads#L275), the file is opened as text instead of binary. I think the following patch would fix the issue: if (dev->pcap_filename) { - int fd = qemu_open_old(dev->pcap_filename, O_CREAT | O_WRONLY | O_TRUNC, 0666); + int fd = qemu_open_old(dev->pcap_filename, O_CREAT | O_WRONLY | O_TRUNC | O_BINARY, 0666); if (fd < 0) { error_setg(errp, "open %s failed", dev->pcap_filename); usb_qdev_unrealize(qdev); return; } - dev->pcap = fdopen(fd, "w"); + dev->pcap = fdopen(fd, "wb"); usb_pcap_init(dev->pcap); } To show an example, when using a very common protocol to USB disks, the BBB protocol uses a 10-byte command packet. For example, the READ_CAPACITY(10) command (implemented at https://gitlab.com/qemu- project/qemu/-/blob/master/hw/scsi/scsi-disk.c#L2068) will have a command block length of 10 (0xA). When this 10-byte command (part of the 31-byte CBW) is placed into the PCAP file, the Windows file manager inserts a 0xD before the 0xA, turning the 31-byte CBW into a 32-byte CBW. Actual CBW: 0040 55 53 42 43 01 00 00 00 08 00 00 00 80 00 0a 25 USBC 0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 %.. PCAP CBW 0040 55 53 42 43 01 00 00 00 08 00 00 00 80 00 0d 0a USBC 0050 25 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 %.. I believe simply opening the PCAP file as BINARY instead of TEXT will fix this issue. Thank you. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/2054889/+subscriptions
Re: [External] Re: [PATCH v2 3/7] migration/multifd: Zero page transmission on the multifd thread.
On Thu, Feb 22, 2024 at 8:38 PM Hao Xiang wrote: > > On Fri, Feb 16, 2024 at 9:08 PM Richard Henderson > wrote: > > > > On 2/16/24 12:39, Hao Xiang wrote: > > > +void multifd_zero_page_check_recv(MultiFDRecvParams *p) > > > +{ > > > +for (int i = 0; i < p->zero_num; i++) { > > > +void *page = p->host + p->zero[i]; > > > +if (!buffer_is_zero(page, p->page_size)) { > > > +memset(page, 0, p->page_size); > > > +} > > > +} > > > +} > > > > You should not check the buffer is zero here, you should just zero it. > > I will fix it in the next version. I tested with zero out all pages but the performance is bad compared to previously. In my test case, most pages are zero pages. I think what happened is that the destination host already has the pages being zero so performing a memcmp is much faster than memset on all zero pages. > > > > > > > r~
[PATCH v4 3/3] tests/qtest: Add testcase for BCM2835 BSC
Simple testcase for validating proper operation of read and write for all three BSC controllers. Signed-off-by: Rayhan Faizel Reviewed-by: Peter Maydell --- tests/qtest/bcm2835-i2c-test.c | 115 + tests/qtest/meson.build| 2 +- 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 tests/qtest/bcm2835-i2c-test.c diff --git a/tests/qtest/bcm2835-i2c-test.c b/tests/qtest/bcm2835-i2c-test.c new file mode 100644 index 00..513ecce61d --- /dev/null +++ b/tests/qtest/bcm2835-i2c-test.c @@ -0,0 +1,115 @@ +/* + * QTest testcase for Broadcom Serial Controller (BSC) + * + * Copyright (c) 2024 Rayhan Faizel + * + * SPDX-License-Identifier: MIT + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "libqtest-single.h" + +#include "hw/i2c/bcm2835_i2c.h" +#include "hw/sensor/tmp105_regs.h" + +static const uint32_t bsc_base_addrs[] = { +0x3f205000, /* I2C0 */ +0x3f804000, /* I2C1 */ +0x3f805000, /* I2C2 */ +}; + +static void bcm2835_i2c_init_transfer(uint32_t base_addr, bool read) +{ +/* read flag is bit 0 so we can write it directly */ +int interrupt = read ? BCM2835_I2C_C_INTR : BCM2835_I2C_C_INTT; + +writel(base_addr + BCM2835_I2C_C, + BCM2835_I2C_C_I2CEN | BCM2835_I2C_C_INTD | + BCM2835_I2C_C_ST | BCM2835_I2C_C_CLEAR | interrupt | read); +} + +static void test_i2c_read_write(gconstpointer data) +{ +uint32_t i2cdata; +intptr_t index = (intptr_t) data; +uint32_t base_addr = bsc_base_addrs[index]; + +/* Write to TMP105 register */ +writel(base_addr + BCM2835_I2C_A, 0x50); +writel(base_addr + BCM2835_I2C_DLEN, 3); + +bcm2835_i2c_init_transfer(base_addr, 0); + +writel(base_addr + BCM2835_I2C_FIFO, TMP105_REG_T_HIGH); +writel(base_addr + BCM2835_I2C_FIFO, 0xde); +writel(base_addr + BCM2835_I2C_FIFO, 0xad); + +/* Clear flags */ +writel(base_addr + BCM2835_I2C_S, BCM2835_I2C_S_DONE | BCM2835_I2C_S_ERR | + BCM2835_I2C_S_CLKT); + +/* Read from TMP105 register */ +writel(base_addr + BCM2835_I2C_A, 0x50); +writel(base_addr + BCM2835_I2C_DLEN, 1); + +bcm2835_i2c_init_transfer(base_addr, 0); + +writel(base_addr + BCM2835_I2C_FIFO, TMP105_REG_T_HIGH); + +writel(base_addr + BCM2835_I2C_DLEN, 2); +bcm2835_i2c_init_transfer(base_addr, 1); + +i2cdata = readl(base_addr + BCM2835_I2C_FIFO); +g_assert_cmpint(i2cdata, ==, 0xde); + +i2cdata = readl(base_addr + BCM2835_I2C_FIFO); +g_assert_cmpint(i2cdata, ==, 0xad); + +/* Clear flags */ +writel(base_addr + BCM2835_I2C_S, BCM2835_I2C_S_DONE | BCM2835_I2C_S_ERR | + BCM2835_I2C_S_CLKT); + +} + +int main(int argc, char **argv) +{ +int ret; +int i; + +g_test_init(&argc, &argv, NULL); + +for (i = 0; i < 3; i++) { +g_autofree char *test_name = +g_strdup_printf("/bcm2835/bcm2835-i2c%d/read_write", i); +qtest_add_data_func(test_name, (void *)(intptr_t) i, +test_i2c_read_write); +} + +/* Run I2C tests with TMP105 slaves on all three buses */ +qtest_start("-M raspi3b " +"-device tmp105,address=0x50,bus=i2c-bus.0 " +"-device tmp105,address=0x50,bus=i2c-bus.1 " +"-device tmp105,address=0x50,bus=i2c-bus.2"); +ret = g_test_run(); +qtest_end(); + +return ret; +} diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 39557d5ecb..8fe303160e 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -228,7 +228,7 @@ qtests_aarch64 = \ ['tpm-tis-device-test', 'tpm-tis-device-swtpm-test'] : []) + \ (config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test', 'fuzz-xlnx-dp-te
[PATCH v4 2/3] hw/arm: Connect BSC to BCM2835 board as I2C0, I2C1 and I2C2
BCM2835 has three I2C controllers. All of them share the same interrupt line. Signed-off-by: Rayhan Faizel --- hw/arm/Kconfig | 1 + hw/arm/bcm2835_peripherals.c | 45 ++-- include/hw/arm/bcm2835_peripherals.h | 4 ++- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 980b14d58d..2b52cec980 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -430,6 +430,7 @@ config RASPI select SDHCI select USB_DWC2 select BCM2835_SPI +select BCM2835_I2C config STM32F100_SOC bool diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c index d5573fd954..9642df22cf 100644 --- a/hw/arm/bcm2835_peripherals.c +++ b/hw/arm/bcm2835_peripherals.c @@ -30,6 +30,9 @@ #define SEPARATE_DMA_IRQ_MAX 10 #define ORGATED_DMA_IRQ_COUNT 4 +/* All three I2C controllers share the same IRQ */ +#define ORGATED_I2C_IRQ_COUNT 3 + static void create_unimp(BCM2835PeripheralState *ps, UnimplementedDeviceState *uds, const char *name, hwaddr ofs, hwaddr size) @@ -148,6 +151,19 @@ static void bcm2835_peripherals_init(Object *obj) /* SPI */ object_initialize_child(obj, "bcm2835-spi0", &s->spi[0], TYPE_BCM2835_SPI); + +/* I2C */ +object_initialize_child(obj, "bcm2835-i2c0", &s->i2c[0], +TYPE_BCM2835_I2C); +object_initialize_child(obj, "bcm2835-i2c1", &s->i2c[1], +TYPE_BCM2835_I2C); +object_initialize_child(obj, "bcm2835-i2c2", &s->i2c[2], +TYPE_BCM2835_I2C); + +object_initialize_child(obj, "orgated-i2c-irq", +&s->orgated_i2c_irq, TYPE_OR_IRQ); +object_property_set_int(OBJECT(&s->orgated_i2c_irq), "num-lines", +ORGATED_I2C_IRQ_COUNT, &error_abort); } static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp) @@ -418,14 +434,37 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp) BCM2835_IC_GPU_IRQ, INTERRUPT_SPI)); +/* I2C */ +for (n = 0; n < 3; n++) { +if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c[n]), errp)) { +return; +} +} + +memory_region_add_subregion(&s->peri_mr, BSC0_OFFSET, +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[0]), 0)); +memory_region_add_subregion(&s->peri_mr, BSC1_OFFSET, +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[1]), 0)); +memory_region_add_subregion(&s->peri_mr, BSC2_OFFSET, +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[2]), 0)); + +if (!qdev_realize(DEVICE(&s->orgated_i2c_irq), NULL, errp)) { +return; +} +for (n = 0; n < ORGATED_I2C_IRQ_COUNT; n++) { +sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c[n]), 0, + qdev_get_gpio_in(DEVICE(&s->orgated_i2c_irq), n)); +} +qdev_connect_gpio_out(DEVICE(&s->orgated_i2c_irq), 0, + qdev_get_gpio_in_named(DEVICE(&s->ic), + BCM2835_IC_GPU_IRQ, + INTERRUPT_I2C)); + create_unimp(s, &s->txp, "bcm2835-txp", TXP_OFFSET, 0x1000); create_unimp(s, &s->armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 0x40); create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100); create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100); create_unimp(s, &s->bscsl, "bcm2835-spis", BSC_SL_OFFSET, 0x100); -create_unimp(s, &s->i2c[0], "bcm2835-i2c0", BSC0_OFFSET, 0x20); -create_unimp(s, &s->i2c[1], "bcm2835-i2c1", BSC1_OFFSET, 0x20); -create_unimp(s, &s->i2c[2], "bcm2835-i2c2", BSC2_OFFSET, 0x20); create_unimp(s, &s->otp, "bcm2835-otp", OTP_OFFSET, 0x80); create_unimp(s, &s->dbus, "bcm2835-dbus", DBUS_OFFSET, 0x8000); create_unimp(s, &s->ave0, "bcm2835-ave0", AVE0_OFFSET, 0x8000); diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h index 0203bb79d8..09a3c06533 100644 --- a/include/hw/arm/bcm2835_peripherals.h +++ b/include/hw/arm/bcm2835_peripherals.h @@ -32,6 +32,7 @@ #include "hw/timer/bcm2835_systmr.h" #include "hw/usb/hcd-dwc2.h" #include "hw/ssi/bcm2835_spi.h" +#include "hw/i2c/bcm2835_i2c.h" #include "hw/misc/unimp.h" #include "qom/object.h" @@ -68,7 +69,8 @@ struct BCM2835PeripheralState { Bcm2835ThermalState thermal; UnimplementedDeviceState i2s; BCM2835SPIState spi[1]; -UnimplementedDeviceState i2c[3]; +BCM2835I2CState i2c[3]; +OrIRQState orgated_i2c_irq; UnimplementedDeviceState otp; UnimplementedDeviceState dbus; UnimplementedDeviceState ave0; -- 2.34.1
[PATCH v4 1/3] hw/i2c: Implement Broadcom Serial Controller (BSC)
A few deficiencies in the current device model need to be noted. 1. FIFOs are not used. All sends and receives are done directly. 2. Repeated starts are not emulated. Repeated starts can be triggered in real hardware by sending a new read transfer request in the window time between transfer active set of write transfer request and done bit set of the same. Signed-off-by: Rayhan Faizel Reviewed-by: Peter Maydell --- docs/system/arm/raspi.rst| 1 + hw/i2c/Kconfig | 4 + hw/i2c/bcm2835_i2c.c | 282 +++ hw/i2c/meson.build | 1 + include/hw/i2c/bcm2835_i2c.h | 80 ++ 5 files changed, 368 insertions(+) create mode 100644 hw/i2c/bcm2835_i2c.c create mode 100644 include/hw/i2c/bcm2835_i2c.h diff --git a/docs/system/arm/raspi.rst b/docs/system/arm/raspi.rst index d0a6f08b2b..f2c0d6d6b8 100644 --- a/docs/system/arm/raspi.rst +++ b/docs/system/arm/raspi.rst @@ -34,6 +34,7 @@ Implemented devices * MailBox controller (MBOX) * VideoCore firmware (property) * Peripheral SPI controller (SPI) + * Broadcom Serial Controller (I2C) Missing devices diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig index 14886b35da..596a7a3165 100644 --- a/hw/i2c/Kconfig +++ b/hw/i2c/Kconfig @@ -45,3 +45,7 @@ config PCA954X config PMBUS bool select SMBUS + +config BCM2835_I2C +bool +select I2C diff --git a/hw/i2c/bcm2835_i2c.c b/hw/i2c/bcm2835_i2c.c new file mode 100644 index 00..20ec46eeab --- /dev/null +++ b/hw/i2c/bcm2835_i2c.c @@ -0,0 +1,282 @@ +/* + * Broadcom Serial Controller (BSC) + * + * Copyright (c) 2024 Rayhan Faizel + * + * SPDX-License-Identifier: MIT + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "hw/i2c/bcm2835_i2c.h" +#include "hw/irq.h" +#include "migration/vmstate.h" + +static void bcm2835_i2c_update_interrupt(BCM2835I2CState *s) +{ +int do_interrupt = 0; +/* Interrupt on RXR (Needs reading) */ +if (s->c & BCM2835_I2C_C_INTR && s->s & BCM2835_I2C_S_RXR) { +do_interrupt = 1; +} + +/* Interrupt on TXW (Needs writing) */ +if (s->c & BCM2835_I2C_C_INTT && s->s & BCM2835_I2C_S_TXW) { +do_interrupt = 1; +} + +/* Interrupt on DONE (Transfer complete) */ +if (s->c & BCM2835_I2C_C_INTD && s->s & BCM2835_I2C_S_DONE) { +do_interrupt = 1; +} +qemu_set_irq(s->irq, do_interrupt); +} + +static void bcm2835_i2c_begin_transfer(BCM2835I2CState *s) +{ +int direction = s->c & BCM2835_I2C_C_READ; +if (i2c_start_transfer(s->bus, s->a, direction)) { +s->s |= BCM2835_I2C_S_ERR; +} +s->s |= BCM2835_I2C_S_TA; + +if (direction) { +s->s |= BCM2835_I2C_S_RXR | BCM2835_I2C_S_RXD; +} else { +s->s |= BCM2835_I2C_S_TXW; +} +} + +static void bcm2835_i2c_finish_transfer(BCM2835I2CState *s) +{ +/* + * STOP is sent when DLEN counts down to zero. + * + * https://github.com/torvalds/linux/blob/v6.7/drivers/i2c/busses/i2c-bcm2835.c#L223-L261 + * It is possible to initiate repeated starts on real hardware. + * However, this requires sending another ST request before the bytes in + * TX FIFO are shifted out. + * + * This is not emulated currently. + */ +i2c_end_transfer(s->bus); +s->s |= BCM2835_I2C_S_DONE; + +/* Ensure RXD is cleared, otherwise the driver registers an error */ +s->s &= ~(BCM2835_I2C_S_TA | BCM2835_I2C_S_RXR | + BCM2835_I2C_S_TXW | BCM2835_I2C_S_RXD); +} + +static uint64_t bcm2835_i2c_read(void *opaque, hwaddr addr, unsigned size) +{ +BCM2835I2CState *s = opaque; +uint32_t readval = 0; + +switch (addr) { +case BCM2835_I2C_C: +readval = s->c; +break; +case BCM2835_I2C_S: +readval = s->s; +break; +case BCM2835_I2C_DLEN: +readval = s->dlen; +break; +case BCM2835_I2C_A: +
[PATCH v4 0/3] Add support for I2C in BCM2835 boards
This patch series implements support for the Broadcom Serial Controller used by BCM2835 based boards for I2C. [Changes in v4] - Added IRQ or-gate for common BSC IRQ. - Added valid sizes to MemoryRegionOps. - Use version tag instead of master [Changes in v3] - Add SPDX license identifiers. - Fix a few minor whitespace issues. [Changes in v2] - Fixed and simplified writing to status register Resolves: https://gitlab.com/qemu-project/qemu/-/issues/481 Signed-off-by: Rayhan Faizel Rayhan Faizel (3): hw/i2c: Implement Broadcom Serial Controller (BSC) hw/arm: Connect BSC to BCM2835 board as I2C0, I2C1 and I2C2 tests/qtest: Add testcase for BCM2835 BSC docs/system/arm/raspi.rst| 1 + hw/arm/Kconfig | 1 + hw/arm/bcm2835_peripherals.c | 45 - hw/i2c/Kconfig | 4 + hw/i2c/bcm2835_i2c.c | 282 +++ hw/i2c/meson.build | 1 + include/hw/arm/bcm2835_peripherals.h | 4 +- include/hw/i2c/bcm2835_i2c.h | 80 tests/qtest/bcm2835-i2c-test.c | 115 +++ tests/qtest/meson.build | 2 +- 10 files changed, 530 insertions(+), 5 deletions(-) create mode 100644 hw/i2c/bcm2835_i2c.c create mode 100644 include/hw/i2c/bcm2835_i2c.h create mode 100644 tests/qtest/bcm2835-i2c-test.c -- 2.34.1
[PATCH v5 0/3] Add support for I2C in BCM2835 boards
This patch series implements support for the Broadcom Serial Controller used by BCM2835 based boards for I2C. [Changes in v5] - Improper whitespace again. [Changes in v4] - Added IRQ or-gate for common BSC IRQ. - Added valid sizes to MemoryRegionOps. - Use version tag instead of master [Changes in v3] - Add SPDX license identifiers. - Fix a few minor whitespace issues. [Changes in v2] - Fixed and simplified writing to status register Resolves: https://gitlab.com/qemu-project/qemu/-/issues/481 Signed-off-by: Rayhan Faizel Rayhan Faizel (3): hw/i2c: Implement Broadcom Serial Controller (BSC) hw/arm: Connect BSC to BCM2835 board as I2C0, I2C1 and I2C2 tests/qtest: Add testcase for BCM2835 BSC docs/system/arm/raspi.rst| 1 + hw/arm/Kconfig | 1 + hw/arm/bcm2835_peripherals.c | 45 - hw/i2c/Kconfig | 4 + hw/i2c/bcm2835_i2c.c | 282 +++ hw/i2c/meson.build | 1 + include/hw/arm/bcm2835_peripherals.h | 4 +- include/hw/i2c/bcm2835_i2c.h | 80 tests/qtest/bcm2835-i2c-test.c | 115 +++ tests/qtest/meson.build | 2 +- 10 files changed, 530 insertions(+), 5 deletions(-) create mode 100644 hw/i2c/bcm2835_i2c.c create mode 100644 include/hw/i2c/bcm2835_i2c.h create mode 100644 tests/qtest/bcm2835-i2c-test.c -- 2.34.1
[PATCH v5 3/3] tests/qtest: Add testcase for BCM2835 BSC
Simple testcase for validating proper operation of read and write for all three BSC controllers. Signed-off-by: Rayhan Faizel Reviewed-by: Peter Maydell --- tests/qtest/bcm2835-i2c-test.c | 115 + tests/qtest/meson.build| 2 +- 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 tests/qtest/bcm2835-i2c-test.c diff --git a/tests/qtest/bcm2835-i2c-test.c b/tests/qtest/bcm2835-i2c-test.c new file mode 100644 index 00..513ecce61d --- /dev/null +++ b/tests/qtest/bcm2835-i2c-test.c @@ -0,0 +1,115 @@ +/* + * QTest testcase for Broadcom Serial Controller (BSC) + * + * Copyright (c) 2024 Rayhan Faizel + * + * SPDX-License-Identifier: MIT + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "libqtest-single.h" + +#include "hw/i2c/bcm2835_i2c.h" +#include "hw/sensor/tmp105_regs.h" + +static const uint32_t bsc_base_addrs[] = { +0x3f205000, /* I2C0 */ +0x3f804000, /* I2C1 */ +0x3f805000, /* I2C2 */ +}; + +static void bcm2835_i2c_init_transfer(uint32_t base_addr, bool read) +{ +/* read flag is bit 0 so we can write it directly */ +int interrupt = read ? BCM2835_I2C_C_INTR : BCM2835_I2C_C_INTT; + +writel(base_addr + BCM2835_I2C_C, + BCM2835_I2C_C_I2CEN | BCM2835_I2C_C_INTD | + BCM2835_I2C_C_ST | BCM2835_I2C_C_CLEAR | interrupt | read); +} + +static void test_i2c_read_write(gconstpointer data) +{ +uint32_t i2cdata; +intptr_t index = (intptr_t) data; +uint32_t base_addr = bsc_base_addrs[index]; + +/* Write to TMP105 register */ +writel(base_addr + BCM2835_I2C_A, 0x50); +writel(base_addr + BCM2835_I2C_DLEN, 3); + +bcm2835_i2c_init_transfer(base_addr, 0); + +writel(base_addr + BCM2835_I2C_FIFO, TMP105_REG_T_HIGH); +writel(base_addr + BCM2835_I2C_FIFO, 0xde); +writel(base_addr + BCM2835_I2C_FIFO, 0xad); + +/* Clear flags */ +writel(base_addr + BCM2835_I2C_S, BCM2835_I2C_S_DONE | BCM2835_I2C_S_ERR | + BCM2835_I2C_S_CLKT); + +/* Read from TMP105 register */ +writel(base_addr + BCM2835_I2C_A, 0x50); +writel(base_addr + BCM2835_I2C_DLEN, 1); + +bcm2835_i2c_init_transfer(base_addr, 0); + +writel(base_addr + BCM2835_I2C_FIFO, TMP105_REG_T_HIGH); + +writel(base_addr + BCM2835_I2C_DLEN, 2); +bcm2835_i2c_init_transfer(base_addr, 1); + +i2cdata = readl(base_addr + BCM2835_I2C_FIFO); +g_assert_cmpint(i2cdata, ==, 0xde); + +i2cdata = readl(base_addr + BCM2835_I2C_FIFO); +g_assert_cmpint(i2cdata, ==, 0xad); + +/* Clear flags */ +writel(base_addr + BCM2835_I2C_S, BCM2835_I2C_S_DONE | BCM2835_I2C_S_ERR | + BCM2835_I2C_S_CLKT); + +} + +int main(int argc, char **argv) +{ +int ret; +int i; + +g_test_init(&argc, &argv, NULL); + +for (i = 0; i < 3; i++) { +g_autofree char *test_name = +g_strdup_printf("/bcm2835/bcm2835-i2c%d/read_write", i); +qtest_add_data_func(test_name, (void *)(intptr_t) i, +test_i2c_read_write); +} + +/* Run I2C tests with TMP105 slaves on all three buses */ +qtest_start("-M raspi3b " +"-device tmp105,address=0x50,bus=i2c-bus.0 " +"-device tmp105,address=0x50,bus=i2c-bus.1 " +"-device tmp105,address=0x50,bus=i2c-bus.2"); +ret = g_test_run(); +qtest_end(); + +return ret; +} diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 39557d5ecb..8fe303160e 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -228,7 +228,7 @@ qtests_aarch64 = \ ['tpm-tis-device-test', 'tpm-tis-device-swtpm-test'] : []) + \ (config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test', 'fuzz-xlnx-dp-te
[PATCH v5 1/3] hw/i2c: Implement Broadcom Serial Controller (BSC)
A few deficiencies in the current device model need to be noted. 1. FIFOs are not used. All sends and receives are done directly. 2. Repeated starts are not emulated. Repeated starts can be triggered in real hardware by sending a new read transfer request in the window time between transfer active set of write transfer request and done bit set of the same. Signed-off-by: Rayhan Faizel Reviewed-by: Peter Maydell --- docs/system/arm/raspi.rst| 1 + hw/i2c/Kconfig | 4 + hw/i2c/bcm2835_i2c.c | 282 +++ hw/i2c/meson.build | 1 + include/hw/i2c/bcm2835_i2c.h | 80 ++ 5 files changed, 368 insertions(+) create mode 100644 hw/i2c/bcm2835_i2c.c create mode 100644 include/hw/i2c/bcm2835_i2c.h diff --git a/docs/system/arm/raspi.rst b/docs/system/arm/raspi.rst index d0a6f08b2b..f2c0d6d6b8 100644 --- a/docs/system/arm/raspi.rst +++ b/docs/system/arm/raspi.rst @@ -34,6 +34,7 @@ Implemented devices * MailBox controller (MBOX) * VideoCore firmware (property) * Peripheral SPI controller (SPI) + * Broadcom Serial Controller (I2C) Missing devices diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig index 14886b35da..596a7a3165 100644 --- a/hw/i2c/Kconfig +++ b/hw/i2c/Kconfig @@ -45,3 +45,7 @@ config PCA954X config PMBUS bool select SMBUS + +config BCM2835_I2C +bool +select I2C diff --git a/hw/i2c/bcm2835_i2c.c b/hw/i2c/bcm2835_i2c.c new file mode 100644 index 00..20ec46eeab --- /dev/null +++ b/hw/i2c/bcm2835_i2c.c @@ -0,0 +1,282 @@ +/* + * Broadcom Serial Controller (BSC) + * + * Copyright (c) 2024 Rayhan Faizel + * + * SPDX-License-Identifier: MIT + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "hw/i2c/bcm2835_i2c.h" +#include "hw/irq.h" +#include "migration/vmstate.h" + +static void bcm2835_i2c_update_interrupt(BCM2835I2CState *s) +{ +int do_interrupt = 0; +/* Interrupt on RXR (Needs reading) */ +if (s->c & BCM2835_I2C_C_INTR && s->s & BCM2835_I2C_S_RXR) { +do_interrupt = 1; +} + +/* Interrupt on TXW (Needs writing) */ +if (s->c & BCM2835_I2C_C_INTT && s->s & BCM2835_I2C_S_TXW) { +do_interrupt = 1; +} + +/* Interrupt on DONE (Transfer complete) */ +if (s->c & BCM2835_I2C_C_INTD && s->s & BCM2835_I2C_S_DONE) { +do_interrupt = 1; +} +qemu_set_irq(s->irq, do_interrupt); +} + +static void bcm2835_i2c_begin_transfer(BCM2835I2CState *s) +{ +int direction = s->c & BCM2835_I2C_C_READ; +if (i2c_start_transfer(s->bus, s->a, direction)) { +s->s |= BCM2835_I2C_S_ERR; +} +s->s |= BCM2835_I2C_S_TA; + +if (direction) { +s->s |= BCM2835_I2C_S_RXR | BCM2835_I2C_S_RXD; +} else { +s->s |= BCM2835_I2C_S_TXW; +} +} + +static void bcm2835_i2c_finish_transfer(BCM2835I2CState *s) +{ +/* + * STOP is sent when DLEN counts down to zero. + * + * https://github.com/torvalds/linux/blob/v6.7/drivers/i2c/busses/i2c-bcm2835.c#L223-L261 + * It is possible to initiate repeated starts on real hardware. + * However, this requires sending another ST request before the bytes in + * TX FIFO are shifted out. + * + * This is not emulated currently. + */ +i2c_end_transfer(s->bus); +s->s |= BCM2835_I2C_S_DONE; + +/* Ensure RXD is cleared, otherwise the driver registers an error */ +s->s &= ~(BCM2835_I2C_S_TA | BCM2835_I2C_S_RXR | + BCM2835_I2C_S_TXW | BCM2835_I2C_S_RXD); +} + +static uint64_t bcm2835_i2c_read(void *opaque, hwaddr addr, unsigned size) +{ +BCM2835I2CState *s = opaque; +uint32_t readval = 0; + +switch (addr) { +case BCM2835_I2C_C: +readval = s->c; +break; +case BCM2835_I2C_S: +readval = s->s; +break; +case BCM2835_I2C_DLEN: +readval = s->dlen; +break; +case BCM2835_I2C_A: +
[PATCH v5 2/3] hw/arm: Connect BSC to BCM2835 board as I2C0, I2C1 and I2C2
BCM2835 has three I2C controllers. All of them share the same interrupt line. Signed-off-by: Rayhan Faizel --- hw/arm/Kconfig | 1 + hw/arm/bcm2835_peripherals.c | 45 ++-- include/hw/arm/bcm2835_peripherals.h | 4 ++- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 980b14d58d..2b52cec980 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -430,6 +430,7 @@ config RASPI select SDHCI select USB_DWC2 select BCM2835_SPI +select BCM2835_I2C config STM32F100_SOC bool diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c index d5573fd954..f6069b23f6 100644 --- a/hw/arm/bcm2835_peripherals.c +++ b/hw/arm/bcm2835_peripherals.c @@ -30,6 +30,9 @@ #define SEPARATE_DMA_IRQ_MAX 10 #define ORGATED_DMA_IRQ_COUNT 4 +/* All three I2C controllers share the same IRQ */ +#define ORGATED_I2C_IRQ_COUNT 3 + static void create_unimp(BCM2835PeripheralState *ps, UnimplementedDeviceState *uds, const char *name, hwaddr ofs, hwaddr size) @@ -148,6 +151,19 @@ static void bcm2835_peripherals_init(Object *obj) /* SPI */ object_initialize_child(obj, "bcm2835-spi0", &s->spi[0], TYPE_BCM2835_SPI); + +/* I2C */ +object_initialize_child(obj, "bcm2835-i2c0", &s->i2c[0], +TYPE_BCM2835_I2C); +object_initialize_child(obj, "bcm2835-i2c1", &s->i2c[1], +TYPE_BCM2835_I2C); +object_initialize_child(obj, "bcm2835-i2c2", &s->i2c[2], +TYPE_BCM2835_I2C); + +object_initialize_child(obj, "orgated-i2c-irq", +&s->orgated_i2c_irq, TYPE_OR_IRQ); +object_property_set_int(OBJECT(&s->orgated_i2c_irq), "num-lines", +ORGATED_I2C_IRQ_COUNT, &error_abort); } static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp) @@ -418,14 +434,37 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp) BCM2835_IC_GPU_IRQ, INTERRUPT_SPI)); +/* I2C */ +for (n = 0; n < 3; n++) { +if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c[n]), errp)) { +return; +} +} + +memory_region_add_subregion(&s->peri_mr, BSC0_OFFSET, +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[0]), 0)); +memory_region_add_subregion(&s->peri_mr, BSC1_OFFSET, +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[1]), 0)); +memory_region_add_subregion(&s->peri_mr, BSC2_OFFSET, +sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->i2c[2]), 0)); + +if (!qdev_realize(DEVICE(&s->orgated_i2c_irq), NULL, errp)) { +return; +} +for (n = 0; n < ORGATED_I2C_IRQ_COUNT; n++) { +sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c[n]), 0, + qdev_get_gpio_in(DEVICE(&s->orgated_i2c_irq), n)); +} +qdev_connect_gpio_out(DEVICE(&s->orgated_i2c_irq), 0, + qdev_get_gpio_in_named(DEVICE(&s->ic), + BCM2835_IC_GPU_IRQ, + INTERRUPT_I2C)); + create_unimp(s, &s->txp, "bcm2835-txp", TXP_OFFSET, 0x1000); create_unimp(s, &s->armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 0x40); create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100); create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100); create_unimp(s, &s->bscsl, "bcm2835-spis", BSC_SL_OFFSET, 0x100); -create_unimp(s, &s->i2c[0], "bcm2835-i2c0", BSC0_OFFSET, 0x20); -create_unimp(s, &s->i2c[1], "bcm2835-i2c1", BSC1_OFFSET, 0x20); -create_unimp(s, &s->i2c[2], "bcm2835-i2c2", BSC2_OFFSET, 0x20); create_unimp(s, &s->otp, "bcm2835-otp", OTP_OFFSET, 0x80); create_unimp(s, &s->dbus, "bcm2835-dbus", DBUS_OFFSET, 0x8000); create_unimp(s, &s->ave0, "bcm2835-ave0", AVE0_OFFSET, 0x8000); diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h index 0203bb79d8..09a3c06533 100644 --- a/include/hw/arm/bcm2835_peripherals.h +++ b/include/hw/arm/bcm2835_peripherals.h @@ -32,6 +32,7 @@ #include "hw/timer/bcm2835_systmr.h" #include "hw/usb/hcd-dwc2.h" #include "hw/ssi/bcm2835_spi.h" +#include "hw/i2c/bcm2835_i2c.h" #include "hw/misc/unimp.h" #include "qom/object.h" @@ -68,7 +69,8 @@ struct BCM2835PeripheralState { Bcm2835ThermalState thermal; UnimplementedDeviceState i2s; BCM2835SPIState spi[1]; -UnimplementedDeviceState i2c[3]; +BCM2835I2CState i2c[3]; +OrIRQState orgated_i2c_irq; UnimplementedDeviceState otp; UnimplementedDeviceState dbus; UnimplementedDeviceState ave0; -- 2.34.1
Re: [PULL v2 00/39] tcg and linux-user patch queue
On 2/24/24 06:15, Peter Maydell wrote: Hi -- looks like this introduces an new variable-length-array, which we are trying to get rid of: ../linux-user/elfload.c: In function 'vma_dump_size': ../linux-user/elfload.c:4254:9: error: ISO C90 forbids variable length array 'page' [-Werror=vla] 4254 | char page[TARGET_PAGE_SIZE]; | ^~~~ ../linux-user/elfload.c: In function 'elf_core_dump': ../linux-user/elfload.c:4778:13: error: ISO C90 forbids variable length array 'page' [-Werror=vla] 4778 | char page[TARGET_PAGE_SIZE]; | ^~~~ I noticed this because I happened to test merging this pullreq together with Thomas's testing pullreq that enforces the -Wvla error. I'll be merging that testing pull shortly but it's not upstream quite yet. Ok, please merge Thomas' first and I'll fix this up. It looks like we should be dynamically allocating these anyway. r~
Re: [PATCH 05/10] hppa: do not require CONFIG_USB
On Fri, Feb 23, 2024 at 8:56 PM BALATON Zoltan wrote: > >> -if (!lasi_dev && machine->enable_graphics) { > >> +if (!lasi_dev && machine->enable_graphics && defaults_enabled()) { > > > > Do we need the defaults_enabled() here? Isn't enable_graphics already > > disabled if defaults_enabled() is not set? > > Isn't enable_graphics controlled by -nographic and defaults_enabled > controlled by -nodefaults? I think they are independent but maybe that > does not answer the question if it's needed or not. Indeed they are different. The idea is that if graphics are disabled you interact with a serial console so you don't need keyboard or mouse; and if default devices are disabled of course you create as little as possible and leave everything to the user. Paolo
Re: [PATCH 2/2] xen: fix stubdom PCI addr
On Mon, Feb 19, 2024 at 1:49 PM Marek Marczykowski-Górecki wrote: > > From: Frédéric Pierret (fepitre) > > When running in a stubdomain, the config space access via sysfs needs to > use BDF as seen inside stubdomain (connected via xen-pcifront), which is > different from the real BDF. For other purposes (hypercall parameters > etc), the real BDF needs to be used. > Get the in-stubdomain BDF by looking up relevant PV PCI xenstore > entries. > > Signed-off-by: Marek Marczykowski-Górecki Anthony made these comments on a different version of this patch: https://lore.kernel.org/xen-devel/48c55d33-aa16-4867-a477-f6df45c7d9d9@perard/ (Sorry I lost track of addressing them at the time.) Regards, Jason
Re: [External] Re: [PATCH v2 3/7] migration/multifd: Zero page transmission on the multifd thread.
On Thu, Feb 22, 2024 at 9:15 PM Hao Xiang wrote: > > On Thu, Feb 22, 2024 at 6:21 PM Peter Xu wrote: > > > > On Wed, Feb 21, 2024 at 06:04:10PM -0300, Fabiano Rosas wrote: > > > Hao Xiang writes: > > > > > > > 1. Implements the zero page detection and handling on the multifd > > > > threads for non-compression, zlib and zstd compression backends. > > > > 2. Added a new value 'multifd' in ZeroPageDetection enumeration. > > > > 3. Add proper asserts to ensure pages->normal are used for normal pages > > > > in all scenarios. > > > > > > > > Signed-off-by: Hao Xiang > > > > --- > > > > migration/meson.build | 1 + > > > > migration/multifd-zero-page.c | 59 +++ > > > > migration/multifd-zlib.c | 26 --- > > > > migration/multifd-zstd.c | 25 --- > > > > migration/multifd.c | 50 +++-- > > > > migration/multifd.h | 7 + > > > > qapi/migration.json | 4 ++- > > > > 7 files changed, 151 insertions(+), 21 deletions(-) > > > > create mode 100644 migration/multifd-zero-page.c > > > > > > > > diff --git a/migration/meson.build b/migration/meson.build > > > > index 92b1cc4297..1eeb915ff6 100644 > > > > --- a/migration/meson.build > > > > +++ b/migration/meson.build > > > > @@ -22,6 +22,7 @@ system_ss.add(files( > > > >'migration.c', > > > >'multifd.c', > > > >'multifd-zlib.c', > > > > + 'multifd-zero-page.c', > > > >'ram-compress.c', > > > >'options.c', > > > >'postcopy-ram.c', > > > > diff --git a/migration/multifd-zero-page.c > > > > b/migration/multifd-zero-page.c > > > > new file mode 100644 > > > > index 00..f0cd8e2c53 > > > > --- /dev/null > > > > +++ b/migration/multifd-zero-page.c > > > > @@ -0,0 +1,59 @@ > > > > +/* > > > > + * Multifd zero page detection implementation. > > > > + * > > > > + * Copyright (c) 2024 Bytedance Inc > > > > + * > > > > + * Authors: > > > > + * Hao Xiang > > > > + * > > > > + * This work is licensed under the terms of the GNU GPL, version 2 or > > > > later. > > > > + * See the COPYING file in the top-level directory. > > > > + */ > > > > + > > > > +#include "qemu/osdep.h" > > > > +#include "qemu/cutils.h" > > > > +#include "exec/ramblock.h" > > > > +#include "migration.h" > > > > +#include "multifd.h" > > > > +#include "options.h" > > > > +#include "ram.h" > > > > + > > > > +void multifd_zero_page_check_send(MultiFDSendParams *p) > > > > +{ > > > > +/* > > > > + * QEMU older than 9.0 don't understand zero page > > > > + * on multifd channel. This switch is required to > > > > + * maintain backward compatibility. > > > > + */ > > > > +bool use_multifd_zero_page = > > > > +(migrate_zero_page_detection() == ZERO_PAGE_DETECTION_MULTIFD); > > > > +MultiFDPages_t *pages = p->pages; > > > > +RAMBlock *rb = pages->block; > > > > + > > > > +assert(pages->num != 0); > > > > +assert(pages->normal_num == 0); > > > > +assert(pages->zero_num == 0); > > > > > > We can drop these before the final version. > > > > > > > + > > > > +for (int i = 0; i < pages->num; i++) { > > > > +uint64_t offset = pages->offset[i]; > > > > +if (use_multifd_zero_page && > > > > +buffer_is_zero(rb->host + offset, p->page_size)) { > > > > +pages->zero[pages->zero_num] = offset; > > > > +pages->zero_num++; > > > > +ram_release_page(rb->idstr, offset); > > > > +} else { > > > > +pages->normal[pages->normal_num] = offset; > > > > +pages->normal_num++; > > > > +} > > > > +} > > > > > > I don't think it's super clean to have three arrays offset, zero and > > > normal, all sized for the full packet size. It might be possible to just > > > carry a bitmap of non-zero pages along with pages->offset and operate on > > > that instead. > > > > > > What do you think? > > > > > > Peter, any ideas? Should we just leave this for another time? > > > > Yeah I think a bitmap should save quite a few fields indeed, it'll however > > make the latter iteration slightly harder by walking both (offset[], > > bitmap), process the page only if bitmap is set for the offset. > > > > IIUC we perhaps don't even need a bitmap? AFAIU what we only need in > > Multifdpages_t is one extra field to mark "how many normal pages", aka, > > normal_num here (zero_num can be calculated from num-normal_num). Then > > the zero page detection logic should do two things: > > > > - Sort offset[] array so that it starts with normal pages, followed up by > > zero pages > > > > - Setup normal_num to be the number of normal pages > > > > Then we reduce 2 new arrays (normal[], zero[]) + 2 new fields (normal_num, > > zero_num) -> 1 new field (normal_num). It'll also be trivial to fill the > > packet header later because offset[] is exactly that. > > > > Side note - I still think it's confusing to read this patch and previous >
Re: [External] Re: [PATCH v2 4/7] migration/multifd: Enable zero page checking from multifd threads.
On Thu, Feb 22, 2024 at 10:02 PM Hao Xiang wrote: > > On Thu, Feb 22, 2024 at 6:33 PM Peter Xu wrote: > > > > On Wed, Feb 21, 2024 at 06:06:19PM -0300, Fabiano Rosas wrote: > > > Hao Xiang writes: > > > > > > > This change adds a dedicated handler for > > > > MigrationOps::ram_save_target_page in > > > > > > nit: Add a dedicated handler... > > > > > > Usually "this patch/change" is used only when necessary to avoid > > > ambiguity. > > > > > > > multifd live migration. Now zero page checking can be done in the > > > > multifd threads > > > > and this becomes the default configuration. We still provide backward > > > > compatibility > > > > where zero page checking is done from the migration main thread. > > > > > > > > Signed-off-by: Hao Xiang > > > > --- > > > > migration/multifd.c | 1 + > > > > migration/options.c | 2 +- > > > > migration/ram.c | 53 ++--- > > > > 3 files changed, 42 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/migration/multifd.c b/migration/multifd.c > > > > index fbb40ea10b..ef5dad1019 100644 > > > > --- a/migration/multifd.c > > > > +++ b/migration/multifd.c > > > > @@ -13,6 +13,7 @@ > > > > #include "qemu/osdep.h" > > > > #include "qemu/cutils.h" > > > > > > This include... > > > > > > > #include "qemu/rcu.h" > > > > +#include "qemu/cutils.h" > > > > > > is there already. > > > > > > > #include "exec/target_page.h" > > > > #include "sysemu/sysemu.h" > > > > #include "exec/ramblock.h" > > > > diff --git a/migration/options.c b/migration/options.c > > > > index 3c603391b0..3c79b6ccd4 100644 > > > > --- a/migration/options.c > > > > +++ b/migration/options.c > > > > @@ -181,7 +181,7 @@ Property migration_properties[] = { > > > >MIG_MODE_NORMAL), > > > > DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", > > > > MigrationState, > > > > parameters.zero_page_detection, > > > > - ZERO_PAGE_DETECTION_LEGACY), > > > > + ZERO_PAGE_DETECTION_MULTIFD), > > > > > > I think we'll need something to avoid a 9.0 -> 8.2 migration with this > > > enabled. Otherwise it will go along happily until we get data corruption > > > because the new QEMU didn't send any zero pages on the migration thread > > > and the old QEMU did not look for them in the multifd packet. > > > > It could be even worse, as the new QEMU will only attach "normal" pages > > after the multifd packet, the old QEMU could read more than it could, > > expecting all pages.. > > > > > > > > Perhaps bumping the MULTIFD_VERSION when ZERO_PAGE_DETECTION_MULTIFD is > > > in use. We'd just need to fix the test in the new QEMU to check > > > (msg.version > MULTIFD_VERSION) instead of (msg.version != > > > MULTIFD_VERSION). > > > > IMHO we don't need yet to change MULTIFD_VERSION, what we need is perhaps a > > compat entry in hw_compat_8_2 setting "zero-page-detection" to "legacy". > > We should make sure when "legacy" is set, multifd ran the old protocol > > (zero_num will always be 0, and will be ignored by old QEMUs, IIUC). > > > > One more comment is, when repost please consider split this patch into two; > > The new ram_save_target_page_multifd() hook can be done in another patch, > > AFAIU. > > Sorry, I kept missing this. I will keep telling myself, compatibility > is king. I will set the hw_compat_8_2 setting and make sure to test > migration 9.0 -> 8.2 fails with "multifd" option set. > Will split patches. So I just want to make sure I am coding the right solution. I added setting "zero-page-detection" to "legacy" in hw_compat_8_2 and tested it. The behavior is that if I set machine type to pc-q35-8.2, zero-page-detection will automatically be set to "legacy". But if I set the machine type to pc-q35-9.0, zero-page-detection will be the default value "multifd". However, this doesn't seem to be a hard requirement because I can still override zero-page-detection to multifd on machine type pc-q35-8.2. Is this OK? > > > > > > > > > > > > > > /* Migration capabilities */ > > > > DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), > > > > diff --git a/migration/ram.c b/migration/ram.c > > > > index 5ece9f042e..b088c5a98c 100644 > > > > --- a/migration/ram.c > > > > +++ b/migration/ram.c > > > > @@ -1123,10 +1123,6 @@ static int save_zero_page(RAMState *rs, > > > > PageSearchStatus *pss, > > > > QEMUFile *file = pss->pss_channel; > > > > int len = 0; > > > > > > > > -if (migrate_zero_page_detection() != ZERO_PAGE_DETECTION_LEGACY) { > > > > -return 0; > > > > -} > > > > > > How does 'none' work now? > > > > > > > - > > > > if (!buffer_is_zero(p, TARGET_PAGE_SIZE)) { > > > > return 0; > > > > } > > > > @@ -1256,6 +1252,10 @@ static int ram_save_page(RAMState *rs, > > > > PageSearchStatus *pss) > > > > > > > > static int ram_save_multifd_page(RAMBlock *block, ram_addr_t offset) > > > > { > > > > +
[PATCH v2] target/riscv: Fix shift count overflow
The result of (8 - 3 - vlmul) is negtive when vlmul >= 6, and results in wrong vill. Signed-off-by: demin.han --- target/riscv/vector_helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index 84cec73eb2..fe56c007d5 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -44,6 +44,7 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1, target_ulong reserved = s2 & MAKE_64BIT_MASK(R_VTYPE_RESERVED_SHIFT, xlen - 1 - R_VTYPE_RESERVED_SHIFT); +uint16_t vlen = cpu->cfg.vlenb << 3; int8_t lmul; if (vlmul & 4) { @@ -53,10 +54,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1, * VLEN * LMUL >= SEW * VLEN >> (8 - lmul) >= sew * (vlenb << 3) >> (8 - lmul) >= sew - * vlenb >> (8 - 3 - lmul) >= sew */ -if (vlmul == 4 || -cpu->cfg.vlenb >> (8 - 3 - vlmul) < sew) { +if (vlmul == 4 || (vlen >> (8 - vlmul)) < sew) { vill = true; } } -- 2.43.2
[PATCH] migration: Free argv
exec_start_outgoing_migration() and exec_start_incoming_migration() leak argv because it uses g_steal_pointer() is used to pass argv qio_channel_command_new_spawn() while it does not free argv either. Removing g_steal_pointer() is not sufficient though because argv is typed g_auto(GStrv), which means the array of strings *and strings* will be freed. The strings are only borrowed from the caller of exec_start_outgoing_migration() and exec_start_incoming_migration() so freeing them result in double-free. Instead, type argv as g_autofree char **. This ensures only the array of strings will be freed and the strings won't be freed. Also, remove unnecessary casts according to the new type. Fixes: cbab4face57b ("migration: convert exec backend to accept MigrateAddress.") Signed-off-by: Akihiko Odaki --- migration/exec.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/migration/exec.c b/migration/exec.c index 47d2f3b8fb02..205675265ea1 100644 --- a/migration/exec.c +++ b/migration/exec.c @@ -73,15 +73,15 @@ void exec_start_outgoing_migration(MigrationState *s, strList *command, QIOChannel *ioc; int length = str_list_length(command); -g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1); +g_autofree char **argv = g_new0(char *, length + 1); init_exec_array(command, argv, errp); -g_autofree char *new_command = g_strjoinv(" ", (char **)argv); +g_autofree char *new_command = g_strjoinv(" ", argv); trace_migration_exec_outgoing(new_command); ioc = QIO_CHANNEL( qio_channel_command_new_spawn( -(const char * const *) g_steal_pointer(&argv), +(const char * const *) argv, O_RDWR, errp)); if (!ioc) { @@ -107,15 +107,15 @@ void exec_start_incoming_migration(strList *command, Error **errp) QIOChannel *ioc; int length = str_list_length(command); -g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1); +g_autofree char **argv = g_new0(char *, length + 1); init_exec_array(command, argv, errp); -g_autofree char *new_command = g_strjoinv(" ", (char **)argv); +g_autofree char *new_command = g_strjoinv(" ", argv); trace_migration_exec_incoming(new_command); ioc = QIO_CHANNEL( qio_channel_command_new_spawn( -(const char * const *) g_steal_pointer(&argv), +(const char * const *) argv, O_RDWR, errp)); if (!ioc) { --- base-commit: 5005aed8a7e728d028efb40e243ecfc2b4f3df3a change-id: 20240219-argv-518065e20e87 Best regards, -- Akihiko Odaki
Re: [PATCH v2 21/27] plugins: add an API to read registers
On 2024/02/24 1:21, Alex Bennée wrote: We can only request a list of registers once the vCPU has been initialised so the user needs to use either call the get function on vCPU initialisation or during the translation phase. We don't expose the reg number to the plugin instead hiding it behind an opaque handle. As the register set is potentially different for each vCPU we store a separate set of handles for each vCPU. This will become more important if we are able to emulate more heterogeneous systems. Having an internal state within the plugins also allows us to expand the interface in future (for example providing callbacks on register change if the translator can track changes). Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706 Cc: Akihiko Odaki Message-Id: <20240103173349.398526-39-alex.ben...@linaro.org> Based-on: <20231025093128.33116-18-akihiko.od...@daynix.com> Signed-off-by: Alex Bennée Reviewed-by: Pierrick Bouvier --- v4 - the get/read_registers functions are now implicitly for current vCPU only to accidental cpu != current_cpu uses. v5 - make reg_handles as per-CPUPluginState variable. --- include/qemu/plugin.h| 2 + include/qemu/qemu-plugin.h | 48 +++- plugins/api.c| 88 plugins/qemu-plugins.symbols | 2 + 4 files changed, 138 insertions(+), 2 deletions(-) diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h index 0e7b9693d80..64fb425fb0b 100644 --- a/include/qemu/plugin.h +++ b/include/qemu/plugin.h @@ -189,9 +189,11 @@ struct qemu_plugin_insn *qemu_plugin_tb_insn_get(struct qemu_plugin_tb *tb, /** * struct CPUPluginState - per-CPU state for plugins * @event_mask: plugin event bitmap. Modified only via async work. + * @reg_handles: hash table of register handles. */ struct CPUPluginState { DECLARE_BITMAP(event_mask, QEMU_PLUGIN_EV_MAX); +GHashTable *reg_handles; }; /** diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index 93981f8f89f..3b6b18058d2 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -11,6 +11,7 @@ #ifndef QEMU_QEMU_PLUGIN_H #define QEMU_QEMU_PLUGIN_H +#include #include #include #include @@ -229,8 +230,8 @@ struct qemu_plugin_insn; * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs * - * Note: currently unused, plugins cannot read or change system - * register state. + * Note: currently QEMU_PLUGIN_CB_RW_REGS is unused, plugins cannot change + * system register state. */ enum qemu_plugin_cb_flags { QEMU_PLUGIN_CB_NO_REGS, @@ -707,4 +708,47 @@ uint64_t qemu_plugin_end_code(void); QEMU_PLUGIN_API uint64_t qemu_plugin_entry_code(void); +/** struct qemu_plugin_register - Opaque handle for register access */ +struct qemu_plugin_register; + +/** + * typedef qemu_plugin_reg_descriptor - register descriptions + * + * @handle: opaque handle for retrieving value with qemu_plugin_read_register + * @name: register name + * @feature: optional feature descriptor, can be NULL + */ +typedef struct { +struct qemu_plugin_register *handle; +const char *name; +const char *feature; +} qemu_plugin_reg_descriptor; + +/** + * qemu_plugin_get_registers() - return register list for current vCPU + * + * Returns a GArray of qemu_plugin_reg_descriptor or NULL. Caller + * frees the array (but not the const strings). + * + * Should be used from a qemu_plugin_register_vcpu_init_cb() callback + * after the vCPU is initialised, i.e. in the vCPU context. + */ +GArray *qemu_plugin_get_registers(void); + +/** + * qemu_plugin_read_register() - read register for current vCPU + * + * @handle: a @qemu_plugin_reg_handle handle + * @buf: A GByteArray for the data owned by the plugin + * + * This function is only available in a context that register read access is + * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag. + * + * Returns the size of the read register. The content of @buf is in target byte + * order. On failure returns -1 + */ +int qemu_plugin_read_register(struct qemu_plugin_register *handle, + GByteArray *buf); + + #endif /* QEMU_QEMU_PLUGIN_H */ diff --git a/plugins/api.c b/plugins/api.c index 54df72c1c00..f6c3ba2366f 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -8,6 +8,7 @@ * * qemu_plugin_tb * qemu_plugin_insn + * qemu_plugin_register * * Which can then be passed back into the API to do additional things. * As such all the public functions in here are exported in @@ -35,10 +36,12 @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" #include "qemu/plugin.h" #include "qemu/log.h" #include "tcg/tcg.h" #include "exec/exec-all.h" +#include "exec/gdbstub.h" #include "exec/ram_addr.h" #include "disas/disas.h" #include "plugin.h" @@ -410,3 +413,88 @@ uint64_t qemu_plugin_entry_code(void) #endif
[PATCH v7 01/16] hw/nvme: Use pcie_sriov_num_vfs()
nvme_sriov_pre_write_ctrl() used to directly inspect SR-IOV configurations to know the number of VFs being disabled due to SR-IOV configuration writes, but the logic was flawed and resulted in out-of-bound memory access. It assumed PCI_SRIOV_NUM_VF always has the number of currently enabled VFs, but it actually doesn't in the following cases: - PCI_SRIOV_NUM_VF has been set but PCI_SRIOV_CTRL_VFE has never been. - PCI_SRIOV_NUM_VF was written after PCI_SRIOV_CTRL_VFE was set. - VFs were only partially enabled because of realization failure. It is a responsibility of pcie_sriov to interpret SR-IOV configurations and pcie_sriov does it correctly, so use pcie_sriov_num_vfs(), which it provides, to get the number of enabled VFs before and after SR-IOV configuration writes. Cc: qemu-sta...@nongnu.org Fixes: CVE-2024-26328 Fixes: 11871f53ef8e ("hw/nvme: Add support for the Virtualization Management command") Suggested-by: Michael S. Tsirkin Signed-off-by: Akihiko Odaki --- hw/nvme/ctrl.c | 26 -- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index f026245d1e9e..7a56e7b79b4d 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -8466,36 +8466,26 @@ static void nvme_pci_reset(DeviceState *qdev) nvme_ctrl_reset(n, NVME_RESET_FUNCTION); } -static void nvme_sriov_pre_write_ctrl(PCIDevice *dev, uint32_t address, - uint32_t val, int len) +static void nvme_sriov_post_write_config(PCIDevice *dev, uint16_t old_num_vfs) { NvmeCtrl *n = NVME(dev); NvmeSecCtrlEntry *sctrl; -uint16_t sriov_cap = dev->exp.sriov_cap; -uint32_t off = address - sriov_cap; -int i, num_vfs; +int i; -if (!sriov_cap) { -return; -} - -if (range_covers_byte(off, len, PCI_SRIOV_CTRL)) { -if (!(val & PCI_SRIOV_CTRL_VFE)) { -num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF); -for (i = 0; i < num_vfs; i++) { -sctrl = &n->sec_ctrl_list.sec[i]; -nvme_virt_set_state(n, le16_to_cpu(sctrl->scid), false); -} -} +for (i = pcie_sriov_num_vfs(dev); i < old_num_vfs; i++) { +sctrl = &n->sec_ctrl_list.sec[i]; +nvme_virt_set_state(n, le16_to_cpu(sctrl->scid), false); } } static void nvme_pci_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len) { -nvme_sriov_pre_write_ctrl(dev, address, val, len); +uint16_t old_num_vfs = pcie_sriov_num_vfs(dev); + pci_default_write_config(dev, address, val, len); pcie_cap_flr_write_config(dev, address, val, len); +nvme_sriov_post_write_config(dev, old_num_vfs); } static const VMStateDescription nvme_vmstate = { -- 2.43.2
[PATCH v7 03/16] pcie_sriov: Reset SR-IOV extended capability
pcie_sriov_pf_disable_vfs() is called when resetting the PF, but it only disables VFs and does not reset SR-IOV extended capability, leaking the state and making the VF Enable register inconsistent with the actual state. Replace pcie_sriov_pf_disable_vfs() with pcie_sriov_pf_reset(), which does not only disable VFs but also resets the capability. Signed-off-by: Akihiko Odaki --- include/hw/pci/pcie_sriov.h | 4 ++-- hw/net/igb.c| 2 +- hw/nvme/ctrl.c | 2 +- hw/pci/pcie_sriov.c | 26 ++ 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h index 095fb0c9edf9..b77eb7bf58ac 100644 --- a/include/hw/pci/pcie_sriov.h +++ b/include/hw/pci/pcie_sriov.h @@ -58,8 +58,8 @@ void pcie_sriov_pf_add_sup_pgsize(PCIDevice *dev, uint16_t opt_sup_pgsize); void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, uint32_t val, int len); -/* Reset SR/IOV VF Enable bit to unregister all VFs */ -void pcie_sriov_pf_disable_vfs(PCIDevice *dev); +/* Reset SR/IOV */ +void pcie_sriov_pf_reset(PCIDevice *dev); /* Get logical VF number of a VF - only valid for VFs */ uint16_t pcie_sriov_vf_number(PCIDevice *dev); diff --git a/hw/net/igb.c b/hw/net/igb.c index 0b5c31a58bba..9345506f81ec 100644 --- a/hw/net/igb.c +++ b/hw/net/igb.c @@ -493,7 +493,7 @@ static void igb_qdev_reset_hold(Object *obj) trace_e1000e_cb_qdev_reset_hold(); -pcie_sriov_pf_disable_vfs(d); +pcie_sriov_pf_reset(d); igb_core_reset(&s->core); } diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 7a56e7b79b4d..7c0d3f108724 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -7116,7 +7116,7 @@ static void nvme_ctrl_reset(NvmeCtrl *n, NvmeResetType rst) } if (rst != NVME_RESET_CONTROLLER) { -pcie_sriov_pf_disable_vfs(pci_dev); +pcie_sriov_pf_reset(pci_dev); } } diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index da209b7f47fd..51b66d1bb342 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -249,16 +249,26 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, } -/* Reset SR/IOV VF Enable bit to trigger an unregister of all VFs */ -void pcie_sriov_pf_disable_vfs(PCIDevice *dev) +/* Reset SR/IOV */ +void pcie_sriov_pf_reset(PCIDevice *dev) { uint16_t sriov_cap = dev->exp.sriov_cap; -if (sriov_cap) { -uint32_t val = pci_get_byte(dev->config + sriov_cap + PCI_SRIOV_CTRL); -if (val & PCI_SRIOV_CTRL_VFE) { -val &= ~PCI_SRIOV_CTRL_VFE; -pcie_sriov_config_write(dev, sriov_cap + PCI_SRIOV_CTRL, val, 1); -} +if (!sriov_cap) { +return; +} + +pci_set_word(dev->config + sriov_cap + PCI_SRIOV_CTRL, 0); +unregister_vfs(dev); + +/* + * Default is to use 4K pages, software can modify it + * to any of the supported bits + */ +pci_set_word(dev->config + sriov_cap + PCI_SRIOV_SYS_PGSIZE, 0x1); + +for (uint16_t i = 0; i < PCI_NUM_REGIONS; i++) { +pci_set_quad(dev->config + sriov_cap + PCI_SRIOV_BAR + i * 4, + dev->exp.sriov_pf.vf_bar_type[i]); } } -- 2.43.2
[PATCH v7 14/16] hw/pci: Determine if rombar is explicitly enabled
vfio determines if rombar is explicitly enabled by inspecting QDict. Inspecting QDict is not nice because QDict is untyped and depends on the details on the external interface. Add an infrastructure to determine if rombar is explicitly enabled to hw/pci. Signed-off-by: Akihiko Odaki --- include/hw/pci/pci_device.h | 5 + 1 file changed, 5 insertions(+) diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h index ca151325085d..6be0f989ebe0 100644 --- a/include/hw/pci/pci_device.h +++ b/include/hw/pci/pci_device.h @@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev) return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn); } +static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev) +{ +return dev->rom_bar && dev->rom_bar != UINT32_MAX; +} + static inline void pci_set_power(PCIDevice *pci_dev, bool state) { /* -- 2.43.2
[PATCH v7 16/16] hw/qdev: Remove opts member
It is no longer used. Signed-off-by: Akihiko Odaki Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Markus Armbruster --- include/hw/qdev-core.h | 4 hw/core/qdev.c | 1 - system/qdev-monitor.c | 12 +++- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 9228e96c87e9..5954404dcbfe 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -237,10 +237,6 @@ struct DeviceState { * @pending_deleted_expires_ms: optional timeout for deletion events */ int64_t pending_deleted_expires_ms; -/** - * @opts: QDict of options for the device - */ -QDict *opts; /** * @hotplugged: was device added after PHASE_MACHINE_READY? */ diff --git a/hw/core/qdev.c b/hw/core/qdev.c index c68d0f7c512f..7349c9a86be8 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -706,7 +706,6 @@ static void device_finalize(Object *obj) dev->canonical_path = NULL; } -qobject_unref(dev->opts); g_free(dev->id); } diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c index a13db763e5dd..71c00f62ee38 100644 --- a/system/qdev-monitor.c +++ b/system/qdev-monitor.c @@ -625,6 +625,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, char *id; DeviceState *dev = NULL; BusState *bus = NULL; +QDict *properties; driver = qdict_get_try_str(opts, "driver"); if (!driver) { @@ -705,13 +706,14 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, } /* set properties */ -dev->opts = qdict_clone_shallow(opts); -qdict_del(dev->opts, "driver"); -qdict_del(dev->opts, "bus"); -qdict_del(dev->opts, "id"); +properties = qdict_clone_shallow(opts); +qdict_del(properties, "driver"); +qdict_del(properties, "bus"); +qdict_del(properties, "id"); -object_set_properties_from_keyval(&dev->parent_obj, dev->opts, from_json, +object_set_properties_from_keyval(&dev->parent_obj, properties, from_json, errp); +qobject_unref(properties); if (*errp) { goto err_del_dev; } -- 2.43.2
[PATCH v7 00/16] hw/pci: SR-IOV related fixes and improvements
I submitted a RFC series[1] to add support for SR-IOV emulation to virtio-net-pci. During the development of the series, I fixed some trivial bugs and made improvements that I think are independently useful. This series extracts those fixes and improvements from the RFC series. [1]: https://patchew.org/QEMU/20231210-sriov-v2-0-b959e8a6d...@daynix.com/ Signed-off-by: Akihiko Odaki --- Changes in v7: - Replaced -1 with UINT32_MAX when expressing uint32_t. (Markus Armbruster) - Added patch "hw/pci: Replace -1 with UINT32_MAX for romsize". - Link to v6: https://lore.kernel.org/r/20240220-reuse-v6-0-2e42a28b0...@daynix.com Changes in v6: - Fixed migration. - Added patch "pcie_sriov: Do not manually unrealize". - Restored patch "pcie_sriov: Release VFs failed to realize" that was missed in v5. - Link to v5: https://lore.kernel.org/r/20240218-reuse-v5-0-e4fc1c19b...@daynix.com Changes in v5: - Added patch "hw/pci: Always call pcie_sriov_pf_reset()". - Added patch "pcie_sriov: Reset SR-IOV extended capability". - Removed a reference to PCI_SRIOV_CTRL_VFE in hw/nvme. (Michael S. Tsirkin) - Noted the impact on the guest of patch "pcie_sriov: Do not reset NumVFs after unregistering VFs". (Michael S. Tsirkin) - Changed to use pcie_sriov_num_vfs(). - Restored pci_set_power() and changed it to call pci_set_enabled() only for PFs with an expalanation. (Michael S. Tsirkin) - Reordered patches. - Link to v4: https://lore.kernel.org/r/20240214-reuse-v4-0-89ad093a0...@daynix.com Changes in v4: - Reverted the change to pci_rom_bar_explicitly_enabled(). (Michael S. Tsirkin) - Added patch "pcie_sriov: Do not reset NumVFs after unregistering VFs". - Added patch "hw/nvme: Refer to dev->exp.sriov_pf.num_vfs". - Link to v3: https://lore.kernel.org/r/20240212-reuse-v3-0-8017b689c...@daynix.com Changes in v3: - Extracted patch "hw/pci: Use -1 as a default value for rombar" from patch "hw/pci: Determine if rombar is explicitly enabled" (Philippe Mathieu-Daudé) - Added an audit result of PCIDevice::rom_bar to the message of patch "hw/pci: Use -1 as a default value for rombar" (Philippe Mathieu-Daudé) - Link to v2: https://lore.kernel.org/r/20240210-reuse-v2-0-24ba2a502...@daynix.com Changes in v2: - Reset after enabling a function so that NVMe VF state gets updated. - Link to v1: https://lore.kernel.org/r/20240203-reuse-v1-0-5be8c5ce6...@daynix.com --- Akihiko Odaki (16): hw/nvme: Use pcie_sriov_num_vfs() pcie_sriov: Validate NumVFs pcie_sriov: Reset SR-IOV extended capability pcie_sriov: Do not reset NumVFs after disabling VFs hw/pci: Always call pcie_sriov_pf_reset() hw/pci: Rename has_power to enabled pcie_sriov: Do not manually unrealize pcie_sriov: Reuse SR-IOV VF device instances pcie_sriov: Release VFs failed to realize pcie_sriov: Remove num_vfs from PCIESriovPF pcie_sriov: Register VFs after migration hw/pci: Replace -1 with UINT32_MAX for romsize hw/pci: Use UINT32_MAX as a default value for rombar hw/pci: Determine if rombar is explicitly enabled vfio: Avoid inspecting option QDict for rombar hw/qdev: Remove opts member docs/pcie_sriov.txt | 8 ++- include/hw/pci/pci.h| 2 +- include/hw/pci/pci_device.h | 22 +- include/hw/pci/pcie_sriov.h | 13 ++-- include/hw/qdev-core.h | 4 -- hw/core/qdev.c | 1 - hw/net/igb.c| 15 ++-- hw/nvme/ctrl.c | 54 +++--- hw/pci/pci.c| 32 + hw/pci/pci_host.c | 4 +- hw/pci/pcie_sriov.c | 170 hw/vfio/pci.c | 3 +- hw/xen/xen_pt_load_rom.c| 2 +- system/qdev-monitor.c | 12 ++-- hw/pci/trace-events | 2 +- 15 files changed, 194 insertions(+), 150 deletions(-) --- base-commit: 5005aed8a7e728d028efb40e243ecfc2b4f3df3a change-id: 20240129-reuse-faae22b11934 Best regards, -- Akihiko Odaki
[PATCH v7 04/16] pcie_sriov: Do not reset NumVFs after disabling VFs
The spec does not NumVFs is reset after disabling VFs except when resetting the PF. Clearing it is guest visible and out of spec, even though Linux doesn't rely on this value being preserved, so we never noticed. Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)") Signed-off-by: Akihiko Odaki --- hw/pci/pcie_sriov.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index 51b66d1bb342..e9b23221d713 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -215,7 +215,6 @@ static void unregister_vfs(PCIDevice *dev) g_free(dev->exp.sriov_pf.vf); dev->exp.sriov_pf.vf = NULL; dev->exp.sriov_pf.num_vfs = 0; -pci_set_word(dev->config + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0); } void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, @@ -260,6 +259,8 @@ void pcie_sriov_pf_reset(PCIDevice *dev) pci_set_word(dev->config + sriov_cap + PCI_SRIOV_CTRL, 0); unregister_vfs(dev); +pci_set_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF, 0); + /* * Default is to use 4K pages, software can modify it * to any of the supported bits -- 2.43.2
[PATCH v7 02/16] pcie_sriov: Validate NumVFs
The guest may write NumVFs greater than TotalVFs and that can lead to buffer overflow in VF implementations. Cc: qemu-sta...@nongnu.org Fixes: CVE-2024-26327 Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)") Signed-off-by: Akihiko Odaki --- hw/pci/pcie_sriov.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index a1fe65f5d801..da209b7f47fd 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -176,6 +176,9 @@ static void register_vfs(PCIDevice *dev) assert(sriov_cap > 0); num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF); +if (num_vfs > pci_get_word(dev->config + sriov_cap + PCI_SRIOV_TOTAL_VF)) { +return; +} dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs); -- 2.43.2
[PATCH v7 12/16] hw/pci: Replace -1 with UINT32_MAX for romsize
romsize is an uint32_t variable. Specifying -1 as an uint32_t value is obscure way to denote UINT32_MAX. Worse, if int is wider than 32-bit, it will change the behavior of a construct like the following: romsize = -1; if (romsize != -1) { ... } When -1 is assigned to romsize, -1 will be implicitly casted into uint32_t, resulting in UINT32_MAX. On contrary, when evaluating romsize != -1, romsize will be casted into int, and it will be a comparison of UINT32_MAX and -1, and result in false. Fix these issues by replacing -1 with UINT32_MAX for statements involving the variable. Signed-off-by: Akihiko Odaki --- hw/pci/pci.c | 8 hw/xen/xen_pt_load_rom.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 54b375da2d26..84df07a2789b 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -70,7 +70,7 @@ static bool pcie_has_upstream_port(PCIDevice *dev); static Property pci_props[] = { DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), DEFINE_PROP_STRING("romfile", PCIDevice, romfile), -DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1), +DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, UINT32_MAX), DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false), @@ -2073,7 +2073,7 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) g_cmp_uint32, NULL); } -if (pci_dev->romsize != -1 && !is_power_of_2(pci_dev->romsize)) { +if (pci_dev->romsize != UINT32_MAX && !is_power_of_2(pci_dev->romsize)) { error_setg(errp, "ROM size %u is not a power of two", pci_dev->romsize); return; } @@ -2359,7 +2359,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, return; } -if (load_file || pdev->romsize == -1) { +if (load_file || pdev->romsize == UINT32_MAX) { path = qemu_find_file(QEMU_FILE_TYPE_BIOS, pdev->romfile); if (path == NULL) { path = g_strdup(pdev->romfile); @@ -2378,7 +2378,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, pdev->romfile); return; } -if (pdev->romsize != -1) { +if (pdev->romsize != UINT_MAX) { if (size > pdev->romsize) { error_setg(errp, "romfile \"%s\" (%u bytes) " "is too large for ROM size %u", diff --git a/hw/xen/xen_pt_load_rom.c b/hw/xen/xen_pt_load_rom.c index 03422a8a7148..6bc64acd3352 100644 --- a/hw/xen/xen_pt_load_rom.c +++ b/hw/xen/xen_pt_load_rom.c @@ -53,7 +53,7 @@ void *pci_assign_dev_load_option_rom(PCIDevice *dev, } fseek(fp, 0, SEEK_SET); -if (dev->romsize != -1) { +if (dev->romsize != UINT_MAX) { if (st.st_size > dev->romsize) { error_report("ROM BAR \"%s\" (%ld bytes) is too large for ROM size %u", rom_file, (long) st.st_size, dev->romsize); -- 2.43.2
[PATCH v7 15/16] vfio: Avoid inspecting option QDict for rombar
Use pci_rom_bar_explicitly_enabled() to determine if rombar is explicitly enabled. Signed-off-by: Akihiko Odaki --- hw/vfio/pci.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 4fa387f0430d..647f15b2a060 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1012,7 +1012,6 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev) { uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK); off_t offset = vdev->config_offset + PCI_ROM_ADDRESS; -DeviceState *dev = DEVICE(vdev); char *name; int fd = vdev->vbasedev.fd; @@ -1046,7 +1045,7 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev) } if (vfio_opt_rom_in_denylist(vdev)) { -if (dev->opts && qdict_haskey(dev->opts, "rombar")) { +if (pci_rom_bar_explicitly_enabled(&vdev->pdev)) { warn_report("Device at %s is known to cause system instability" " issues during option rom execution", vdev->vbasedev.name); -- 2.43.2
[PATCH v7 08/16] pcie_sriov: Reuse SR-IOV VF device instances
Disable SR-IOV VF devices by reusing code to power down PCI devices instead of removing them when the guest requests to disable VFs. This allows to realize devices and report VF realization errors at PF realization time. Signed-off-by: Akihiko Odaki --- docs/pcie_sriov.txt | 8 ++-- include/hw/pci/pci.h| 5 --- include/hw/pci/pci_device.h | 15 +++ include/hw/pci/pcie_sriov.h | 6 +-- hw/net/igb.c| 13 -- hw/nvme/ctrl.c | 24 +++ hw/pci/pci.c| 2 +- hw/pci/pcie_sriov.c | 102 +++- 8 files changed, 95 insertions(+), 80 deletions(-) diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt index a47aad0bfab0..ab2142807f79 100644 --- a/docs/pcie_sriov.txt +++ b/docs/pcie_sriov.txt @@ -52,9 +52,11 @@ setting up a BAR for a VF. ... /* Add and initialize the SR/IOV capability */ - pcie_sriov_pf_init(d, 0x200, "your_virtual_dev", - vf_devid, initial_vfs, total_vfs, - fun_offset, stride); + if (!pcie_sriov_pf_init(d, 0x200, "your_virtual_dev", + vf_devid, initial_vfs, total_vfs, + fun_offset, stride, errp)) { + return; + } /* Set up individual VF BARs (parameters as for normal BARs) */ pcie_sriov_pf_init_vf_bar( ... ) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 6c92b2f70008..442017b4865d 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -644,9 +644,4 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev) MSIMessage pci_get_msi_message(PCIDevice *dev, int vector); void pci_set_enabled(PCIDevice *pci_dev, bool state); -static inline void pci_set_power(PCIDevice *pci_dev, bool state) -{ -pci_set_enabled(pci_dev, state); -} - #endif diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h index d57f9ce83884..ca151325085d 100644 --- a/include/hw/pci/pci_device.h +++ b/include/hw/pci/pci_device.h @@ -205,6 +205,21 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev) return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn); } +static inline void pci_set_power(PCIDevice *pci_dev, bool state) +{ +/* + * Don't change the enabled state of VFs when powering on/off the device. + * + * When powering on, VFs must not be enabled immediately but they must + * wait until the guest configures SR-IOV. + * When powering off, their corresponding PFs will be reset and disable + * VFs. + */ +if (!pci_is_vf(pci_dev)) { +pci_set_enabled(pci_dev, state); +} +} + uint16_t pci_requester_id(PCIDevice *dev); /* DMA access functions */ diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h index b77eb7bf58ac..4b1133f79e15 100644 --- a/include/hw/pci/pcie_sriov.h +++ b/include/hw/pci/pcie_sriov.h @@ -18,7 +18,6 @@ struct PCIESriovPF { uint16_t num_vfs; /* Number of virtual functions created */ uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */ -const char *vfname; /* Reference to the device type used for the VFs */ PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */ }; @@ -27,10 +26,11 @@ struct PCIESriovVF { uint16_t vf_number; /* Logical VF number of this function */ }; -void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset, +bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset, const char *vfname, uint16_t vf_dev_id, uint16_t init_vfs, uint16_t total_vfs, -uint16_t vf_offset, uint16_t vf_stride); +uint16_t vf_offset, uint16_t vf_stride, +Error **errp); void pcie_sriov_pf_exit(PCIDevice *dev); /* Set up a VF bar in the SR/IOV bar area */ diff --git a/hw/net/igb.c b/hw/net/igb.c index 9b37523d6df8..907259fd8b3b 100644 --- a/hw/net/igb.c +++ b/hw/net/igb.c @@ -447,9 +447,16 @@ static void igb_pci_realize(PCIDevice *pci_dev, Error **errp) pcie_ari_init(pci_dev, 0x150); -pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, TYPE_IGBVF, -IGB_82576_VF_DEV_ID, IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS, -IGB_VF_OFFSET, IGB_VF_STRIDE); +if (!pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, +TYPE_IGBVF, IGB_82576_VF_DEV_ID, +IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS, +IGB_VF_OFFSET, IGB_VF_STRIDE, +errp)) { +pcie_cap_exit(pci_dev); +igb_cleanup_msix(s); +msi_uninit(pci_dev); +return; +} pcie_sriov_pf_init_vf_bar(pci_dev, IGBVF_MMIO_BAR_IDX, PCI_BASE_ADDRESS_MEM_TYPE_64 | PCI_BASE_ADDRESS_MEM_PREFETCH, diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index c1af4b87b34a..98c3e942077c 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -8027,7
[PATCH v7 09/16] pcie_sriov: Release VFs failed to realize
Release VFs failed to realize just as we do in unregister_vfs(). Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)") Signed-off-by: Akihiko Odaki --- hw/pci/pcie_sriov.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index d934cd7d0e64..8710ee95b26d 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -86,6 +86,8 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset, vf->exp.sriov_vf.vf_number = i; if (!qdev_realize(&vf->qdev, bus, errp)) { +object_unparent(OBJECT(vf)); +object_unref(vf); unparent_vfs(dev, i); return false; } -- 2.43.2
[PATCH v7 11/16] pcie_sriov: Register VFs after migration
pcie_sriov doesn't have code to restore its state after migration, but igb, which uses pcie_sriov, naively claimed its migration capability. Add code to register VFs after migration and fix igb migration. Fixes: 3a977deebe6b ("Intrdocue igb device emulation") Signed-off-by: Akihiko Odaki --- include/hw/pci/pcie_sriov.h | 2 ++ hw/pci/pci.c| 7 +++ hw/pci/pcie_sriov.c | 7 +++ 3 files changed, 16 insertions(+) diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h index 793d03c5f12e..d576a8c6be19 100644 --- a/include/hw/pci/pcie_sriov.h +++ b/include/hw/pci/pcie_sriov.h @@ -57,6 +57,8 @@ void pcie_sriov_pf_add_sup_pgsize(PCIDevice *dev, uint16_t opt_sup_pgsize); void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, uint32_t val, int len); +void pcie_sriov_pf_post_load(PCIDevice *dev); + /* Reset SR/IOV */ void pcie_sriov_pf_reset(PCIDevice *dev); diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 750c2ba696d1..54b375da2d26 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -733,10 +733,17 @@ static bool migrate_is_not_pcie(void *opaque, int version_id) return !pci_is_express((PCIDevice *)opaque); } +static int pci_post_load(void *opaque, int version_id) +{ +pcie_sriov_pf_post_load(opaque); +return 0; +} + const VMStateDescription vmstate_pci_device = { .name = "PCIDevice", .version_id = 2, .minimum_version_id = 1, +.post_load = pci_post_load, .fields = (const VMStateField[]) { VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice), VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice, diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index fb48c1a18c9a..09a53ed30027 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -239,6 +239,13 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, } } +void pcie_sriov_pf_post_load(PCIDevice *dev) +{ +if (dev->exp.sriov_cap) { +register_vfs(dev); +} +} + /* Reset SR/IOV */ void pcie_sriov_pf_reset(PCIDevice *dev) -- 2.43.2
[PATCH v7 05/16] hw/pci: Always call pcie_sriov_pf_reset()
Call pcie_sriov_pf_reset() from pci_do_device_reset() just as we do for msi_reset() and msix_reset() to prevent duplicating code for each SR-IOV PF. Signed-off-by: Akihiko Odaki --- hw/net/igb.c | 2 -- hw/nvme/ctrl.c | 4 hw/pci/pci.c | 1 + 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/hw/net/igb.c b/hw/net/igb.c index 9345506f81ec..9b37523d6df8 100644 --- a/hw/net/igb.c +++ b/hw/net/igb.c @@ -488,12 +488,10 @@ static void igb_pci_uninit(PCIDevice *pci_dev) static void igb_qdev_reset_hold(Object *obj) { -PCIDevice *d = PCI_DEVICE(obj); IGBState *s = IGB(obj); trace_e1000e_cb_qdev_reset_hold(); -pcie_sriov_pf_reset(d); igb_core_reset(&s->core); } diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 7c0d3f108724..c1af4b87b34a 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -7114,10 +7114,6 @@ static void nvme_ctrl_reset(NvmeCtrl *n, NvmeResetType rst) sctrl = &n->sec_ctrl_list.sec[i]; nvme_virt_set_state(n, le16_to_cpu(sctrl->scid), false); } - -if (rst != NVME_RESET_CONTROLLER) { -pcie_sriov_pf_reset(pci_dev); -} } if (rst != NVME_RESET_CONTROLLER) { diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 6496d027ca61..e7a39cb203ae 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -409,6 +409,7 @@ static void pci_do_device_reset(PCIDevice *dev) msi_reset(dev); msix_reset(dev); +pcie_sriov_pf_reset(dev); } /* -- 2.43.2
[PATCH v7 13/16] hw/pci: Use UINT32_MAX as a default value for rombar
Currently there is no way to distinguish the case that rombar is explicitly specified as 1 and the case that rombar is not specified. Set rombar UINT32_MAX by default to distinguish these cases just as it is done for addr and romsize. It was confirmed that changing the default value to UINT32_MAX will not change the behavior by looking at occurences of rom_bar. $ git grep -w rom_bar hw/display/qxl.c:328:QXLRom *rom = memory_region_get_ram_ptr(&d->rom_bar); hw/display/qxl.c:431:qxl_set_dirty(&qxl->rom_bar, 0, qxl->rom_size); hw/display/qxl.c:1048:QXLRom *rom = memory_region_get_ram_ptr(&qxl->rom_bar); hw/display/qxl.c:2131:memory_region_init_rom(&qxl->rom_bar, OBJECT(qxl), "qxl.vrom", hw/display/qxl.c:2154: PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->rom_bar); hw/display/qxl.h:101:MemoryRegion rom_bar; hw/pci/pci.c:74:DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), hw/pci/pci.c:2329:if (!pdev->rom_bar) { hw/vfio/pci.c:1019:if (vdev->pdev.romfile || !vdev->pdev.rom_bar) { hw/xen/xen_pt_load_rom.c:29:if (dev->romfile || !dev->rom_bar) { include/hw/pci/pci_device.h:150:uint32_t rom_bar; rom_bar refers to a different variable in qxl. It is only tested if the value is 0 or not in the other places. This changes the semantics of UINT32_MAX, which has always been a valid value to explicitly say rombar is enabled to denote the implicit default value. Nobody should have been set UINT32_MAX to rombar however, considering that its meaning was no different from 1 and typing a literal UINT32_MAX (0x or 4294967295) is more troublesome. Signed-off-by: Akihiko Odaki --- hw/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 84df07a2789b..cb5ac46e9f27 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -71,7 +71,7 @@ static Property pci_props[] = { DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), DEFINE_PROP_STRING("romfile", PCIDevice, romfile), DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, UINT32_MAX), -DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), +DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, UINT32_MAX), DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false), DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present, -- 2.43.2
[PATCH v7 10/16] pcie_sriov: Remove num_vfs from PCIESriovPF
num_vfs is not migrated so use PCI_SRIOV_CTRL_VFE and PCI_SRIOV_NUM_VF instead. Signed-off-by: Akihiko Odaki --- include/hw/pci/pcie_sriov.h | 1 - hw/pci/pcie_sriov.c | 28 hw/pci/trace-events | 2 +- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h index 4b1133f79e15..793d03c5f12e 100644 --- a/include/hw/pci/pcie_sriov.h +++ b/include/hw/pci/pcie_sriov.h @@ -16,7 +16,6 @@ #include "hw/pci/pci.h" struct PCIESriovPF { -uint16_t num_vfs; /* Number of virtual functions created */ uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */ PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */ }; diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index 8710ee95b26d..fb48c1a18c9a 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -44,7 +44,6 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset, pcie_add_capability(dev, PCI_EXT_CAP_ID_SRIOV, 1, offset, PCI_EXT_CAP_SRIOV_SIZEOF); dev->exp.sriov_cap = offset; -dev->exp.sriov_pf.num_vfs = 0; dev->exp.sriov_pf.vf = NULL; pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset); @@ -173,6 +172,12 @@ void pcie_sriov_vf_register_bar(PCIDevice *dev, int region_num, } } +static void clear_ctrl_vfe(PCIDevice *dev) +{ +uint8_t *ctrl = dev->config + dev->exp.sriov_cap + PCI_SRIOV_CTRL; +pci_set_word(ctrl, pci_get_word(ctrl) & ~PCI_SRIOV_CTRL_VFE); +} + static void register_vfs(PCIDevice *dev) { uint16_t num_vfs; @@ -182,6 +187,7 @@ static void register_vfs(PCIDevice *dev) assert(sriov_cap > 0); num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF); if (num_vfs > pci_get_word(dev->config + sriov_cap + PCI_SRIOV_TOTAL_VF)) { +clear_ctrl_vfe(dev); return; } @@ -190,20 +196,18 @@ static void register_vfs(PCIDevice *dev) for (i = 0; i < num_vfs; i++) { pci_set_enabled(dev->exp.sriov_pf.vf[i], true); } -dev->exp.sriov_pf.num_vfs = num_vfs; } static void unregister_vfs(PCIDevice *dev) { -uint16_t num_vfs = dev->exp.sriov_pf.num_vfs; uint16_t i; +uint8_t *cfg = dev->config + dev->exp.sriov_cap; trace_sriov_unregister_vfs(dev->name, PCI_SLOT(dev->devfn), - PCI_FUNC(dev->devfn), num_vfs); -for (i = 0; i < num_vfs; i++) { + PCI_FUNC(dev->devfn)); +for (i = 0; i < pci_get_word(cfg + PCI_SRIOV_TOTAL_VF); i++) { pci_set_enabled(dev->exp.sriov_pf.vf[i], false); } -dev->exp.sriov_pf.num_vfs = 0; } void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, @@ -229,6 +233,9 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, } else { unregister_vfs(dev); } +} else if (range_covers_byte(off, len, PCI_SRIOV_NUM_VF)) { +clear_ctrl_vfe(dev); +unregister_vfs(dev); } } @@ -291,7 +298,7 @@ PCIDevice *pcie_sriov_get_pf(PCIDevice *dev) PCIDevice *pcie_sriov_get_vf_at_index(PCIDevice *dev, int n) { assert(!pci_is_vf(dev)); -if (n < dev->exp.sriov_pf.num_vfs) { +if (n < pcie_sriov_num_vfs(dev)) { return dev->exp.sriov_pf.vf[n]; } return NULL; @@ -299,5 +306,10 @@ PCIDevice *pcie_sriov_get_vf_at_index(PCIDevice *dev, int n) uint16_t pcie_sriov_num_vfs(PCIDevice *dev) { -return dev->exp.sriov_pf.num_vfs; +uint16_t sriov_cap = dev->exp.sriov_cap; +uint8_t *cfg = dev->config + sriov_cap; + +return sriov_cap && + (pci_get_word(cfg + PCI_SRIOV_CTRL) & PCI_SRIOV_CTRL_VFE) ? + pci_get_word(cfg + PCI_SRIOV_NUM_VF) : 0; } diff --git a/hw/pci/trace-events b/hw/pci/trace-events index 42430869ce05..b30e31619787 100644 --- a/hw/pci/trace-events +++ b/hw/pci/trace-events @@ -14,5 +14,5 @@ msix_write_config(char *name, bool enabled, bool masked) "dev %s enabled %d mask # hw/pci/pcie_sriov.c sriov_register_vfs(const char *name, int slot, int function, int num_vfs) "%s %02x:%x: creating %d vf devs" -sriov_unregister_vfs(const char *name, int slot, int function, int num_vfs) "%s %02x:%x: Unregistering %d vf devs" +sriov_unregister_vfs(const char *name, int slot, int function) "%s %02x:%x: Unregistering vf devs" sriov_config_write(const char *name, int slot, int fun, uint32_t offset, uint32_t val, uint32_t len) "%s %02x:%x: sriov offset 0x%x val 0x%x len %d" -- 2.43.2
[PATCH v7 06/16] hw/pci: Rename has_power to enabled
The renamed state will not only represent powering state of PFs, but also represent SR-IOV VF enablement in the future. Signed-off-by: Akihiko Odaki --- include/hw/pci/pci.h| 7 ++- include/hw/pci/pci_device.h | 2 +- hw/pci/pci.c| 14 +++--- hw/pci/pci_host.c | 4 ++-- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index eaa3fc99d884..6c92b2f70008 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -642,6 +642,11 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev) } MSIMessage pci_get_msi_message(PCIDevice *dev, int vector); -void pci_set_power(PCIDevice *pci_dev, bool state); +void pci_set_enabled(PCIDevice *pci_dev, bool state); + +static inline void pci_set_power(PCIDevice *pci_dev, bool state) +{ +pci_set_enabled(pci_dev, state); +} #endif diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h index d3dd0f64b273..d57f9ce83884 100644 --- a/include/hw/pci/pci_device.h +++ b/include/hw/pci/pci_device.h @@ -56,7 +56,7 @@ typedef struct PCIReqIDCache PCIReqIDCache; struct PCIDevice { DeviceState qdev; bool partially_hotplugged; -bool has_power; +bool enabled; /* PCI config space */ uint8_t *config; diff --git a/hw/pci/pci.c b/hw/pci/pci.c index e7a39cb203ae..8bde13f7cd1e 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1525,7 +1525,7 @@ static void pci_update_mappings(PCIDevice *d) continue; new_addr = pci_bar_address(d, i, r->type, r->size); -if (!d->has_power) { +if (!d->enabled) { new_addr = PCI_BAR_UNMAPPED; } @@ -1613,7 +1613,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val_in, int pci_update_irq_disabled(d, was_irq_disabled); memory_region_set_enabled(&d->bus_master_enable_region, (pci_get_word(d->config + PCI_COMMAND) - & PCI_COMMAND_MASTER) && d->has_power); + & PCI_COMMAND_MASTER) && d->enabled); } msi_write_config(d, addr, val_in, l); @@ -2811,18 +2811,18 @@ MSIMessage pci_get_msi_message(PCIDevice *dev, int vector) return msg; } -void pci_set_power(PCIDevice *d, bool state) +void pci_set_enabled(PCIDevice *d, bool state) { -if (d->has_power == state) { +if (d->enabled == state) { return; } -d->has_power = state; +d->enabled = state; pci_update_mappings(d); memory_region_set_enabled(&d->bus_master_enable_region, (pci_get_word(d->config + PCI_COMMAND) - & PCI_COMMAND_MASTER) && d->has_power); -if (!d->has_power) { + & PCI_COMMAND_MASTER) && d->enabled); +if (!d->enabled) { pci_device_reset(d); } } diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c index dfe6fe618401..0d82727cc9dd 100644 --- a/hw/pci/pci_host.c +++ b/hw/pci/pci_host.c @@ -86,7 +86,7 @@ void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr, * allowing direct removal of unexposed functions. */ if ((pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev)) || -!pci_dev->has_power || is_pci_dev_ejected(pci_dev)) { +!pci_dev->enabled || is_pci_dev_ejected(pci_dev)) { return; } @@ -111,7 +111,7 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr, * allowing direct removal of unexposed functions. */ if ((pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev)) || -!pci_dev->has_power || is_pci_dev_ejected(pci_dev)) { +!pci_dev->enabled || is_pci_dev_ejected(pci_dev)) { return ~0x0; } -- 2.43.2
[PATCH v7 07/16] pcie_sriov: Do not manually unrealize
A device gets automatically unrealized when being unparented. Signed-off-by: Akihiko Odaki --- hw/pci/pcie_sriov.c | 5 - 1 file changed, 5 deletions(-) diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index e9b23221d713..8b1fd2a89ad7 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -17,7 +17,6 @@ #include "hw/qdev-properties.h" #include "qemu/error-report.h" #include "qemu/range.h" -#include "qapi/error.h" #include "trace.h" static PCIDevice *register_vf(PCIDevice *pf, int devfn, @@ -204,11 +203,7 @@ static void unregister_vfs(PCIDevice *dev) trace_sriov_unregister_vfs(dev->name, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn), num_vfs); for (i = 0; i < num_vfs; i++) { -Error *err = NULL; PCIDevice *vf = dev->exp.sriov_pf.vf[i]; -if (!object_property_set_bool(OBJECT(vf), "realized", false, &err)) { -error_reportf_err(err, "Failed to unplug: "); -} object_unparent(OBJECT(vf)); object_unref(OBJECT(vf)); } -- 2.43.2
[PATCH v6 3/3] tests/qtest: Add STM32L4x5 GPIO QTest testcase
The testcase contains : - `test_idr_reset_value()` : Checks the reset values of MODER, OTYPER, PUPDR, ODR and IDR. - `test_gpio_output_mode()` : Checks that writing a bit in register ODR results in the corresponding pin rising or lowering, if this pin is configured in output mode. - `test_gpio_input_mode()` : Checks that a input pin set high or low externally results in the pin rising and lowering. - `test_pull_up_pull_down()` : Checks that a floating pin in pull-up/down mode is actually high/down. - `test_push_pull()` : Checks that a pin set externally is disconnected when configured in push-pull output mode, and can't be set externally while in this mode. - `test_open_drain()` : Checks that a pin set externally high is disconnected when configured in open-drain output mode, and can't be set high while in this mode. - `test_bsrr_brr()` : Checks that writing to BSRR and BRR has the desired result in ODR. - `test_clock_enable()` : Checks that GPIO clock is at the right frequency after enabling it. Acked-by: Thomas Huth Signed-off-by: Arnaud Minier Signed-off-by: Inès Varhol --- tests/qtest/stm32l4x5_gpio-test.c | 586 ++ tests/qtest/meson.build | 3 +- 2 files changed, 588 insertions(+), 1 deletion(-) create mode 100644 tests/qtest/stm32l4x5_gpio-test.c diff --git a/tests/qtest/stm32l4x5_gpio-test.c b/tests/qtest/stm32l4x5_gpio-test.c new file mode 100644 index 00..cd4fd9bae2 --- /dev/null +++ b/tests/qtest/stm32l4x5_gpio-test.c @@ -0,0 +1,586 @@ +/* + * QTest testcase for STM32L4x5_GPIO + * + * Copyright (c) 2024 Arnaud Minier + * Copyright (c) 2024 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 GPIO_BASE_ADDR 0x4800 +#define GPIO_SIZE 0x400 +#define NUM_GPIOS 8 +#define NUM_GPIO_PINS 16 + +#define GPIO_A 0x4800 +#define GPIO_B 0x48000400 +#define GPIO_C 0x48000800 +#define GPIO_D 0x48000C00 +#define GPIO_E 0x48001000 +#define GPIO_F 0x48001400 +#define GPIO_G 0x48001800 +#define GPIO_H 0x48001C00 + +/* + * MSI is used as system clock source after startup + * from Reset, configured at 4 MHz. + */ +#define SYSCLK_FREQ_HZ 400 +#define RCC_AHB2ENR 0x4002104C + +#define MODER 0x00 +#define OTYPER 0x04 +#define PUPDR 0x0C +#define IDR 0x10 +#define ODR 0x14 +#define BSRR 0x18 +#define BRR 0x28 + +#define MODER_INPUT 0 +#define MODER_OUTPUT 1 + +#define PUPDR_NONE 0 +#define PUPDR_PULLUP 1 +#define PUPDR_PULLDOWN 2 + +#define OTYPER_PUSH_PULL 0 +#define OTYPER_OPEN_DRAIN 1 + +const uint32_t moder_reset[NUM_GPIOS] = { +0xABFF, +0xFEBF, +0x, +0x, +0x, +0x, +0x, +0x000F +}; + +const uint32_t pupdr_reset[NUM_GPIOS] = { +0x6400, +0x0100, +0x, +0x, +0x, +0x, +0x, +0x +}; + +const uint32_t idr_reset[NUM_GPIOS] = { +0xA000, +0x0010, +0x, +0x, +0x, +0x, +0x, +0x +}; + +static uint32_t gpio_readl(unsigned int gpio, unsigned int offset) +{ +return readl(gpio + offset); +} + +static void gpio_writel(unsigned int gpio, unsigned int offset, uint32_t value) +{ +writel(gpio + offset, value); +} + +static void gpio_set_bit(unsigned int gpio, unsigned int reg, + unsigned int pin, uint32_t value) +{ +uint32_t mask = 0x & ~(0x1 << pin); +gpio_writel(gpio, reg, (gpio_readl(gpio, reg) & mask) | value << pin); +} + +static void gpio_set_2bits(unsigned int gpio, unsigned int reg, + unsigned int pin, uint32_t value) +{ +uint32_t offset = 2 * pin; +uint32_t mask = 0x & ~(0x3 << offset); +gpio_writel(gpio, reg, (gpio_readl(gpio, reg) & mask) | value << offset); +} + +static unsigned int get_gpio_id(uint32_t gpio_addr) +{ +return (gpio_addr - GPIO_BASE_ADDR) / GPIO_SIZE; +} + +static void gpio_set_irq(unsigned int gpio, int num, int level) +{ +g_autofree char *name = g_strdup_printf("/machine/soc/gpio%c", +get_gpio_id(gpio) + 'a'); +qtest_set_irq_in(global_qtest, name, NULL, num, level); +} + +static void disconnect_all_pins(unsigned int gpio) +{ +g_autofree char *path = g_strdup_printf("/machine/soc/gpio%c", +get_gpio_id(gpio) + 'a'); +QDict *r; + +r = qtest_qmp(global_qtest, "{ 'execute': 'qom-set', 'arguments': " +"{ 'path': %s, 'property': 'disconnected-pins', 'value': %d } }", +path, 0x); +g_assert_false(qdict_haskey(r, "error")); +qobject_unref(r); +} + +static uint32_t get_disconnected_pins(unsigned int gpio) +{ +g_autofree char *path = g_strdup_printf("/machine/soc/gpio%c", +
[PATCH v6 0/3] Add device STM32L4x5 GPIO
This patch adds a new device STM32L4x5 GPIO device and is part of a series implementing the STM32L4x5 with a few peripherals. Changes from v5 : - remove duplicate macro constant `GPIO_NUM_PINS` from syscfg.h (it's defined in gpio.h) - moving definition of constant `NUM_GPIOS` from syscfg.h to gpio.h - soc.c : replacing a hardcoded 16 by the correct `GPIO_NUM_PINS` Changes from v4 : - gpio.c : use helpers `is_pull_up()`, `is_pull_down()`, `is_output()` for more clarity - gpio.c : correct `update_gpio_idr()` in case of open-drain pin set to 1 in ODR and set to 0 externally - gpio.c : rename `get_gpio_pins_to_disconnect()` to `get_gpio_pinmask_to_disconnect()` and associated comments - gpio.c : correct coding style issues (alignment and declaration) - soc.c : unite structs `gpio_addr` and `stm32l4x5_gpio_initval` Changes from v3 : - replacing occurences of '16' with the correct macro `GPIO_NUM_PINS` - updating copyright year - rebasing on latest version of STM32L4x5 RCC Changes from v2 : - correct memory leaks caused by re-assigning a `g_autofree` pointer without freeing it - gpio-test : test that reset values (and not just initialization values) are correct, correct `stm32l4x5_gpio_reset()` accordingly - adding a `clock-freq-hz` object property to test that enabling GPIO clock in RCC sets the GPIO clocks Changes from v1 : - replacing test GPIO register `DISCONNECTED_PINS` with an object property accessed using `qtest_qmp()` in the qtest (through helpers `get_disconnected_pins()` and `disconnect_all_pins()`) - removing GPIO subclasses and storing MODER, OSPEEDR and PUPDR reset values in properties - adding a `name` property and using it for more lisible traces - using `g_strdup_printf()` to facilitate setting irqs in the qtest, and initializing GPIO children in soc_initfn Changes from RFC v1 : - `stm32l4x5-gpio-test.c` : correct typos, make the test generic, add a test for bitwise writing in register ODR - `stm32l4x5_soc.c` : connect gpios to their clock, use an array of GpioState - `stm32l4x5_gpio.c` : correct comments in `update_gpio_idr()`, correct `get_gpio_pins_to_disconnect()`, correct `stm32l4x5_gpio_init()` and initialize the clock, add a realize function - update MAINAINERS Based-on: 20240219200908.49551-1-arnaud.min...@telecom-paris.fr ([PATCH v5 0/8] Add device STM32L4x5 RCC) Signed-off-by: Arnaud Minier Signed-off-by: Inès Varhol Inès Varhol (3): hw/gpio: Implement STM32L4x5 GPIO hw/arm: Connect STM32L4x5 GPIO to STM32L4x5 SoC tests/qtest: Add STM32L4x5 GPIO QTest testcase MAINTAINERS| 1 + docs/system/arm/b-l475e-iot01a.rst | 2 +- include/hw/arm/stm32l4x5_soc.h | 2 + include/hw/gpio/stm32l4x5_gpio.h | 71 include/hw/misc/stm32l4x5_syscfg.h | 3 +- hw/arm/stm32l4x5_soc.c | 71 +++- hw/gpio/stm32l4x5_gpio.c | 477 +++ hw/misc/stm32l4x5_syscfg.c | 1 + tests/qtest/stm32l4x5_gpio-test.c | 586 + hw/arm/Kconfig | 3 +- hw/gpio/Kconfig| 3 + hw/gpio/meson.build| 1 + hw/gpio/trace-events | 6 + tests/qtest/meson.build| 3 +- 14 files changed, 1210 insertions(+), 20 deletions(-) create mode 100644 include/hw/gpio/stm32l4x5_gpio.h create mode 100644 hw/gpio/stm32l4x5_gpio.c create mode 100644 tests/qtest/stm32l4x5_gpio-test.c -- 2.43.2
[PATCH v6 2/3] hw/arm: Connect STM32L4x5 GPIO to STM32L4x5 SoC
Signed-off-by: Arnaud Minier Signed-off-by: Inès Varhol Reviewed-by: Philippe Mathieu-Daudé --- include/hw/arm/stm32l4x5_soc.h | 2 + include/hw/gpio/stm32l4x5_gpio.h | 1 + include/hw/misc/stm32l4x5_syscfg.h | 3 +- hw/arm/stm32l4x5_soc.c | 71 +++--- hw/misc/stm32l4x5_syscfg.c | 1 + hw/arm/Kconfig | 3 +- 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/include/hw/arm/stm32l4x5_soc.h b/include/hw/arm/stm32l4x5_soc.h index 1f71298b45..cb4da08629 100644 --- a/include/hw/arm/stm32l4x5_soc.h +++ b/include/hw/arm/stm32l4x5_soc.h @@ -29,6 +29,7 @@ #include "hw/misc/stm32l4x5_syscfg.h" #include "hw/misc/stm32l4x5_exti.h" #include "hw/misc/stm32l4x5_rcc.h" +#include "hw/gpio/stm32l4x5_gpio.h" #include "qom/object.h" #define TYPE_STM32L4X5_SOC "stm32l4x5-soc" @@ -45,6 +46,7 @@ struct Stm32l4x5SocState { Stm32l4x5ExtiState exti; Stm32l4x5SyscfgState syscfg; Stm32l4x5RccState rcc; +Stm32l4x5GpioState gpio[NUM_GPIOS]; MemoryRegion sram1; MemoryRegion sram2; diff --git a/include/hw/gpio/stm32l4x5_gpio.h b/include/hw/gpio/stm32l4x5_gpio.h index 0d361f3410..878bd19fc9 100644 --- a/include/hw/gpio/stm32l4x5_gpio.h +++ b/include/hw/gpio/stm32l4x5_gpio.h @@ -25,6 +25,7 @@ #define TYPE_STM32L4X5_GPIO "stm32l4x5-gpio" OBJECT_DECLARE_SIMPLE_TYPE(Stm32l4x5GpioState, STM32L4X5_GPIO) +#define NUM_GPIOS 8 #define GPIO_NUM_PINS 16 struct Stm32l4x5GpioState { diff --git a/include/hw/misc/stm32l4x5_syscfg.h b/include/hw/misc/stm32l4x5_syscfg.h index 29c3522f9d..23bb564150 100644 --- a/include/hw/misc/stm32l4x5_syscfg.h +++ b/include/hw/misc/stm32l4x5_syscfg.h @@ -26,12 +26,11 @@ #include "hw/sysbus.h" #include "qom/object.h" +#include "hw/gpio/stm32l4x5_gpio.h" #define TYPE_STM32L4X5_SYSCFG "stm32l4x5-syscfg" OBJECT_DECLARE_SIMPLE_TYPE(Stm32l4x5SyscfgState, STM32L4X5_SYSCFG) -#define NUM_GPIOS 8 -#define GPIO_NUM_PINS 16 #define SYSCFG_NUM_EXTICR 4 struct Stm32l4x5SyscfgState { diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c index 347a5377e5..072671bdfb 100644 --- a/hw/arm/stm32l4x5_soc.c +++ b/hw/arm/stm32l4x5_soc.c @@ -27,6 +27,7 @@ #include "exec/address-spaces.h" #include "sysemu/sysemu.h" #include "hw/arm/stm32l4x5_soc.h" +#include "hw/gpio/stm32l4x5_gpio.h" #include "hw/qdev-clock.h" #include "hw/misc/unimp.h" @@ -78,6 +79,22 @@ static const int exti_irq[NUM_EXTI_IRQ] = { #define RCC_BASE_ADDRESS 0x40021000 #define RCC_IRQ 5 +static const struct { +uint32_t addr; +uint32_t moder_reset; +uint32_t ospeedr_reset; +uint32_t pupdr_reset; +} stm32l4x5_gpio_cfg[NUM_GPIOS] = { +{ 0x4800, 0xABFF, 0x0C00, 0x6400 }, +{ 0x48000400, 0xFEBF, 0x, 0x0100 }, +{ 0x48000800, 0x, 0x, 0x }, +{ 0x48000C00, 0x, 0x, 0x }, +{ 0x48001000, 0x, 0x, 0x }, +{ 0x48001400, 0x, 0x, 0x }, +{ 0x48001800, 0x, 0x, 0x }, +{ 0x48001C00, 0x000F, 0x, 0x }, +}; + static void stm32l4x5_soc_initfn(Object *obj) { Stm32l4x5SocState *s = STM32L4X5_SOC(obj); @@ -85,6 +102,11 @@ 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_STM32L4X5_SYSCFG); object_initialize_child(obj, "rcc", &s->rcc, TYPE_STM32L4X5_RCC); + +for (unsigned i = 0; i < NUM_GPIOS; i++) { +g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i); +object_initialize_child(obj, name, &s->gpio[i], TYPE_STM32L4X5_GPIO); +} } static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp) @@ -93,8 +115,9 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp) Stm32l4x5SocState *s = STM32L4X5_SOC(dev_soc); const Stm32l4x5SocClass *sc = STM32L4X5_SOC_GET_CLASS(dev_soc); MemoryRegion *system_memory = get_system_memory(); -DeviceState *armv7m; +DeviceState *armv7m, *dev; SysBusDevice *busdev; +uint32_t pin_index; if (!memory_region_init_rom(&s->flash, OBJECT(dev_soc), "flash", sc->flash_size, errp)) { @@ -135,17 +158,43 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp) return; } +/* GPIOs */ +for (unsigned i = 0; i < NUM_GPIOS; i++) { +g_autofree char *name = g_strdup_printf("%c", 'A' + i); +dev = DEVICE(&s->gpio[i]); +qdev_prop_set_string(dev, "name", name); +qdev_prop_set_uint32(dev, "mode-reset", + stm32l4x5_gpio_cfg[i].moder_reset); +qdev_prop_set_uint32(dev, "ospeed-reset", + stm32l4x5_gpio_cfg[i].ospeedr_reset); +qdev_prop_set_uint32(dev, "pupd-reset", +stm32l4x5_gpio_cfg[i].
[PATCH v6 1/3] hw/gpio: Implement STM32L4x5 GPIO
Features supported : - the 8 STM32L4x5 GPIOs are initialized with their reset values (except IDR, see below) - input mode : setting a pin in input mode "externally" (using input irqs) results in an out irq (transmitted to SYSCFG) - output mode : setting a bit in ODR sets the corresponding out irq (if this line is configured in output mode) - pull-up, pull-down - push-pull, open-drain Difference with the real GPIOs : - Alternate Function and Analog mode aren't implemented : pins in AF/Analog behave like pins in input mode - floating pins stay at their last value - register IDR reset values differ from the real one : values are coherent with the other registers reset values and the fact that AF/Analog modes aren't implemented - setting I/O output speed isn't supported - locking port bits isn't supported - ADC function isn't supported - GPIOH has 16 pins instead of 2 pins - writing to registers LCKR, AFRL, AFRH and ASCR is ineffective Signed-off-by: Arnaud Minier Signed-off-by: Inès Varhol Reviewed-by: Philippe Mathieu-Daudé --- MAINTAINERS| 1 + docs/system/arm/b-l475e-iot01a.rst | 2 +- include/hw/gpio/stm32l4x5_gpio.h | 70 + hw/gpio/stm32l4x5_gpio.c | 477 + hw/gpio/Kconfig| 3 + hw/gpio/meson.build| 1 + hw/gpio/trace-events | 6 + 7 files changed, 559 insertions(+), 1 deletion(-) create mode 100644 include/hw/gpio/stm32l4x5_gpio.h create mode 100644 hw/gpio/stm32l4x5_gpio.c diff --git a/MAINTAINERS b/MAINTAINERS index 50ab2982bb..cf49c151f3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1131,6 +1131,7 @@ F: hw/arm/stm32l4x5_soc.c F: hw/misc/stm32l4x5_exti.c F: hw/misc/stm32l4x5_syscfg.c F: hw/misc/stm32l4x5_rcc.c +F: hw/gpio/stm32l4x5_gpio.c F: include/hw/*/stm32l4x5_*.h B-L475E-IOT01A IoT Node diff --git a/docs/system/arm/b-l475e-iot01a.rst b/docs/system/arm/b-l475e-iot01a.rst index b857a56ca4..0afef8e4f4 100644 --- a/docs/system/arm/b-l475e-iot01a.rst +++ b/docs/system/arm/b-l475e-iot01a.rst @@ -18,6 +18,7 @@ Currently B-L475E-IOT01A machine's only supports the following devices: - STM32L4x5 EXTI (Extended interrupts and events controller) - STM32L4x5 SYSCFG (System configuration controller) - STM32L4x5 RCC (Reset and clock control) +- STM32L4x5 GPIOs (General-purpose I/Os) Missing devices """ @@ -25,7 +26,6 @@ Missing devices The B-L475E-IOT01A does *not* support the following devices: - Serial ports (UART) -- General-purpose I/Os (GPIO) - Analog to Digital Converter (ADC) - SPI controller - Timer controller (TIMER) diff --git a/include/hw/gpio/stm32l4x5_gpio.h b/include/hw/gpio/stm32l4x5_gpio.h new file mode 100644 index 00..0d361f3410 --- /dev/null +++ b/include/hw/gpio/stm32l4x5_gpio.h @@ -0,0 +1,70 @@ +/* + * STM32L4x5 GPIO (General Purpose Input/Ouput) + * + * Copyright (c) 2024 Arnaud Minier + * Copyright (c) 2024 Inès Varhol + * + * SPDX-License-Identifier: GPL-2.0-or-later + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +/* + * 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 + */ + +#ifndef HW_STM32L4X5_GPIO_H +#define HW_STM32L4X5_GPIO_H + +#include "hw/sysbus.h" +#include "qom/object.h" + +#define TYPE_STM32L4X5_GPIO "stm32l4x5-gpio" +OBJECT_DECLARE_SIMPLE_TYPE(Stm32l4x5GpioState, STM32L4X5_GPIO) + +#define GPIO_NUM_PINS 16 + +struct Stm32l4x5GpioState { +SysBusDevice parent_obj; + +MemoryRegion mmio; + +/* GPIO registers */ +uint32_t moder; +uint32_t otyper; +uint32_t ospeedr; +uint32_t pupdr; +uint32_t idr; +uint32_t odr; +uint32_t lckr; +uint32_t afrl; +uint32_t afrh; +uint32_t ascr; + +/* GPIO registers reset values */ +uint32_t moder_reset; +uint32_t ospeedr_reset; +uint32_t pupdr_reset; + +/* + * External driving of pins. + * The pins can be set externally through the device + * anonymous input GPIOs lines under certain conditions. + * The pin must not be in push-pull output mode, + * and can't be set high in open-drain mode. + * Pins driven externally and configured to + * output mode will in general be "disconnected" + * (see `get_gpio_pinmask_to_disconnect()`) + */ +uint16_t disconnected_pins; +uint16_t pins_connected_high; + +char *name; +Clock *clk; +qemu_irq pin[GPIO_NUM_PINS]; +}; + +#endif diff --git a/hw/gpio/stm32l4x5_gpio.c b/hw/gpio/stm32l4x5_gpio.c new file mode 100644 index 00..63b8763e9d --- /dev/null +++ b/hw/gpio/stm32l4x5_gpio.c @@ -0,0 +1,477 @@ +/* + * STM32L4x5 GPIO (General Purpose Input/Ouput) + * + * Copyright (c) 2024 Arnaud Minier + * Copyright (c) 2024 In
[PATCH v12 02/10] ui/cocoa: Immediately call [-QemuCocoaView handleMouseEvent:buttons:]
Instead of using mouse_event variable to tell to handle a mouse event later, immediately call [-QemuCocoaView handleMouseEvent:buttons:]. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 87 ++ 1 file changed, 30 insertions(+), 57 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 32d61e226507..06bd5737636b 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -799,9 +799,8 @@ - (bool) handleEventLocked:(NSEvent *)event { /* Return true if we handled the event, false if it should be given to OSX */ COCOA_DEBUG("QemuCocoaView: handleEvent\n"); -int buttons = 0; +InputButton button; int keycode = 0; -bool mouse_event = false; // Location of event in virtual screen coordinates NSPoint p = [self screenLocationOfEvent:event]; NSUInteger modifiers = [event modifierFlags]; @@ -947,7 +946,7 @@ - (bool) handleEventLocked:(NSEvent *)event } break; } -break; +return true; case NSEventTypeKeyDown: keycode = cocoa_keycode_to_qemu([event keyCode]); @@ -983,7 +982,7 @@ - (bool) handleEventLocked:(NSEvent *)event } else { [self handleMonitorInput: event]; } -break; +return true; case NSEventTypeKeyUp: keycode = cocoa_keycode_to_qemu([event keyCode]); @@ -996,7 +995,7 @@ - (bool) handleEventLocked:(NSEvent *)event if (qemu_console_is_graphic(NULL)) { qkbd_state_key_event(kbd, keycode, false); } -break; +return true; case NSEventTypeMouseMoved: if (isAbsoluteEnabled) { // Cursor re-entered into a window might generate events bound to screen coordinates @@ -1012,34 +1011,20 @@ - (bool) handleEventLocked:(NSEvent *)event } } } -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:0]; case NSEventTypeLeftMouseDown: -buttons |= MOUSE_EVENT_LBUTTON; -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:MOUSE_EVENT_LBUTTON]; case NSEventTypeRightMouseDown: -buttons |= MOUSE_EVENT_RBUTTON; -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:MOUSE_EVENT_RBUTTON]; case NSEventTypeOtherMouseDown: -buttons |= MOUSE_EVENT_MBUTTON; -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:MOUSE_EVENT_MBUTTON]; case NSEventTypeLeftMouseDragged: -buttons |= MOUSE_EVENT_LBUTTON; -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:MOUSE_EVENT_LBUTTON]; case NSEventTypeRightMouseDragged: -buttons |= MOUSE_EVENT_RBUTTON; -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:MOUSE_EVENT_RBUTTON]; case NSEventTypeOtherMouseDragged: -buttons |= MOUSE_EVENT_MBUTTON; -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:MOUSE_EVENT_MBUTTON]; case NSEventTypeLeftMouseUp: -mouse_event = true; if (!isMouseGrabbed && [self screenContainsPoint:p]) { /* * In fullscreen mode, the window of cocoaView may not be the @@ -1050,53 +1035,41 @@ - (bool) handleEventLocked:(NSEvent *)event [self grabMouse]; } } -break; +return [self handleMouseEvent:event buttons:0]; case NSEventTypeRightMouseUp: -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:0]; case NSEventTypeOtherMouseUp: -mouse_event = true; -break; +return [self handleMouseEvent:event buttons:0]; case NSEventTypeScrollWheel: /* * Send wheel events to the guest regardless of window focus. * This is in-line with standard Mac OS X UI behaviour. */ -/* - * We shouldn't have got a scroll event when deltaY and delta Y - * are zero, hence no harm in dropping the event - */ -if ([event deltaY] != 0 || [event deltaX] != 0) { /* Determine if this is a scroll up or scroll down event */ -if ([event deltaY] != 0) { - buttons = ([event deltaY] > 0) ? +if ([event deltaY] != 0) { +button = ([event deltaY] > 0) ? INPUT_BUTTON_WHEEL_UP : INPUT_BUTTON_WHEEL_DOWN; -} else if ([event delt
[PATCH v12 09/10] ui/cocoa: Call console_select() with the BQL
[-QemuCocoaView displayConsole:] can be called anytime so explicitly take the BQL before it calls console_select(). Signed-off-by: Akihiko Odaki Reviewed-by: Peter Maydell --- ui/cocoa.m | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index cebfae04d9e8..33d31b82bcab 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1360,7 +1360,9 @@ - (void)zoomToFit:(id) sender /* Displays the console on the screen */ - (void)displayConsole:(id)sender { -console_select([sender tag]); +with_bql(^{ +console_select([sender tag]); +}); } /* Pause the guest */ -- 2.43.2
[PATCH v12 03/10] ui/cocoa: Release specific mouse buttons
ui/cocoa used to release all mouse buttons when it sees NSEventTypeLeftMouseUp, NSEventTypeRightMouseUp, or NSEventTypeOtherMouseUp, but it can instead release specific one according to the delivered event. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 36 +++- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 06bd5737636b..c73ef8884454 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -99,7 +99,6 @@ 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; @@ -1011,19 +1010,19 @@ - (bool) handleEventLocked:(NSEvent *)event } } } -return [self handleMouseEvent:event buttons:0]; +return [self handleMouseEvent:event]; case NSEventTypeLeftMouseDown: -return [self handleMouseEvent:event buttons:MOUSE_EVENT_LBUTTON]; +return [self handleMouseEvent:event button:INPUT_BUTTON_LEFT down:true]; case NSEventTypeRightMouseDown: -return [self handleMouseEvent:event buttons:MOUSE_EVENT_RBUTTON]; +return [self handleMouseEvent:event button:INPUT_BUTTON_RIGHT down:true]; case NSEventTypeOtherMouseDown: -return [self handleMouseEvent:event buttons:MOUSE_EVENT_MBUTTON]; +return [self handleMouseEvent:event button:INPUT_BUTTON_MIDDLE down:true]; case NSEventTypeLeftMouseDragged: -return [self handleMouseEvent:event buttons:MOUSE_EVENT_LBUTTON]; +return [self handleMouseEvent:event button:INPUT_BUTTON_LEFT down:true]; case NSEventTypeRightMouseDragged: -return [self handleMouseEvent:event buttons:MOUSE_EVENT_RBUTTON]; +return [self handleMouseEvent:event button:INPUT_BUTTON_RIGHT down:true]; case NSEventTypeOtherMouseDragged: -return [self handleMouseEvent:event buttons:MOUSE_EVENT_MBUTTON]; +return [self handleMouseEvent:event button:INPUT_BUTTON_MIDDLE down:true]; case NSEventTypeLeftMouseUp: if (!isMouseGrabbed && [self screenContainsPoint:p]) { /* @@ -1035,11 +1034,11 @@ - (bool) handleEventLocked:(NSEvent *)event [self grabMouse]; } } -return [self handleMouseEvent:event buttons:0]; +return [self handleMouseEvent:event button:INPUT_BUTTON_LEFT down:false]; case NSEventTypeRightMouseUp: -return [self handleMouseEvent:event buttons:0]; +return [self handleMouseEvent:event button:INPUT_BUTTON_RIGHT down:false]; case NSEventTypeOtherMouseUp: -return [self handleMouseEvent:event buttons:0]; +return [self handleMouseEvent:event button:INPUT_BUTTON_MIDDLE down:false]; case NSEventTypeScrollWheel: /* * Send wheel events to the guest regardless of window focus. @@ -1072,7 +1071,7 @@ - (bool) handleEventLocked:(NSEvent *)event } } -- (bool) handleMouseEvent:(NSEvent *)event buttons:(uint32_t)buttons +- (bool) handleMouseEvent:(NSEvent *)event button:(InputButton)button down:(bool)down { /* Don't send button events to the guest unless we've got a * mouse grab or window focus. If we have neither then this event @@ -1081,17 +1080,12 @@ - (bool) handleMouseEvent:(NSEvent *)event buttons:(uint32_t)buttons * call below. We definitely don't want to pass that click through * to the guest. */ -if ((isMouseGrabbed || [[self window] isKeyWindow]) && -(last_buttons != buttons)) { -static uint32_t bmap[INPUT_BUTTON__MAX] = { -[INPUT_BUTTON_LEFT] = MOUSE_EVENT_LBUTTON, -[INPUT_BUTTON_MIDDLE] = MOUSE_EVENT_MBUTTON, -[INPUT_BUTTON_RIGHT] = MOUSE_EVENT_RBUTTON -}; -qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons); -last_buttons = buttons; +if (!isMouseGrabbed && ![[self window] isKeyWindow]) { +return false; } +qemu_input_queue_btn(dcl.con, button, down); + return [self handleMouseEvent:event]; } -- 2.43.2
[PATCH v12 10/10] ui/cocoa: Remove stretch_video flag
Evaluate [normalWindow styleMask] & NSWindowStyleMaskResizable instead. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 19 ++- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 33d31b82bcab..b33842e70280 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -102,7 +102,6 @@ static void cocoa_switch(DisplayChangeListener *dcl, static int left_command_key_enabled = 1; static bool swap_opt_cmd; -static bool stretch_video; static NSTextField *pauseLabel; static bool allow_events; @@ -514,7 +513,7 @@ - (void) resizeWindow { [[self window] setContentAspectRatio:NSMakeSize(screen.width, screen.height)]; -if (!stretch_video) { +if (!([[self window] styleMask] & NSWindowStyleMaskResizable)) { [[self window] setContentSize:NSMakeSize(screen.width, screen.height)]; [[self window] center]; } else if ([[self window] styleMask] & NSWindowStyleMaskFullScreen) { @@ -1346,15 +1345,10 @@ - (void)showQEMUDoc:(id)sender /* Stretches video to fit host monitor size */ - (void)zoomToFit:(id) sender { -stretch_video = !stretch_video; -if (stretch_video == true) { -[cocoaView window].styleMask |= NSWindowStyleMaskResizable; -[sender setState: NSControlStateValueOn]; -} else { -[cocoaView window].styleMask &= ~NSWindowStyleMaskResizable; -[cocoaView resizeWindow]; -[sender setState: NSControlStateValueOff]; -} +NSWindowStyleMask styleMask = [[cocoaView window] styleMask] ^ NSWindowStyleMaskResizable; + +[[cocoaView window] setStyleMask:styleMask]; +[sender setState:styleMask & NSWindowStyleMaskResizable ? NSControlStateValueOn : NSControlStateValueOff]; } /* Displays the console on the screen */ @@ -1619,7 +1613,7 @@ static void create_initial_menus(void) menu = [[NSMenu alloc] initWithTitle:@"View"]; [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(doToggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen menuItem = [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit" action:@selector(zoomToFit:) keyEquivalent:@""] autorelease]; -[menuItem setState: stretch_video ? NSControlStateValueOn : NSControlStateValueOff]; +[menuItem setState: [[cocoaView window] styleMask] & NSWindowStyleMaskResizable ? NSControlStateValueOn : NSControlStateValueOff]; [menu addItem: menuItem]; menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil keyEquivalent:@""] autorelease]; [menuItem setSubmenu:menu]; @@ -2005,7 +1999,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) } if (opts->u.cocoa.has_zoom_to_fit && opts->u.cocoa.zoom_to_fit) { -stretch_video = true; [cocoaView window].styleMask |= NSWindowStyleMaskResizable; } -- 2.43.2
[PATCH v12 00/10] ui/cocoa: Use NSWindow's ability to resize
V5 -> V6: Rebased. Signed-off-by: Akihiko Odaki --- Changes in v12: - Extracted patches "ui/cocoa: Split [-QemuCocoaView handleEventLocked:]", "ui/cocoa: Immediately call [-QemuCocoaView handleMouseEvent:buttons:]" from "ui/cocoa: Release specific mouse buttons". (Peter Maydell) - Changed the message of patch "ui/cocoa: Call console_select() with the BQL" to refer to [-QemuCocoaView displayConsole:]. (Peter Maydell) - Added patch "ui/cocoa: Fix pause label coordinates". - Added patch "ui/cocoa: Remove normalWindow". - Removed [-QemuCocoaView fixZoomedFullScreenSize:], which turned out unnecessary. - Simplified references to NSWindow.styleMask. - Link to v11: https://lore.kernel.org/r/20240217-cocoa-v11-0-0a17a7e53...@daynix.com Changes in v11: - Ensured to reflect screen size changes to bounds - Link to v10: https://lore.kernel.org/r/20240214-cocoa-v10-0-c7ffe5d9f...@daynix.com Changes in v10: - Removed relative mouse input scaling. - Link to v9: https://lore.kernel.org/r/20240213-cocoa-v9-0-d5a5e1bf0...@daynix.com Changes in v9: - Split patch "ui/cocoa: Use NSWindow's ability to resize" into patches "ui/cocoa: Let the platform toggle fullscreen", "ui/cocoa: Make window resizable", "ui/cocoa: Call console_select() with the BQL". - Added patch "ui/cocoa: Scale with NSView instead of Core Graphics". - Rebased. - Dropped Tested-by: from patch "ui/cocoa: Use NSWindow's ability to resize". - Link to v8: https://lore.kernel.org/r/20231218-cocoa-v8-0-d7fad80c7...@daynix.com Changes in v8: - Split into three patches. (BALATON Zoltan) - Removed negative full-screen conditions. (BALATON Zoltan) - Converted a C++-style comment to C style. - Link to v7: https://lore.kernel.org/r/20231217-cocoa-v7-1-6af21ef75...@daynix.com Changes in v7: - Fixed zoom-to-fit option. (Marek Glogowski) - Link to v6: https://lore.kernel.org/r/20231211-cocoa-v6-1-49f3be019...@daynix.com --- Akihiko Odaki (10): ui/cocoa: Split [-QemuCocoaView handleEventLocked:] ui/cocoa: Immediately call [-QemuCocoaView handleMouseEvent:buttons:] ui/cocoa: Release specific mouse buttons ui/cocoa: Scale with NSView instead of Core Graphics ui/cocoa: Fix pause label coordinates ui/cocoa: Let the platform toggle fullscreen ui/cocoa: Remove normalWindow ui/cocoa: Make window resizable ui/cocoa: Call console_select() with the BQL ui/cocoa: Remove stretch_video flag ui/cocoa.m | 552 +++-- 1 file changed, 242 insertions(+), 310 deletions(-) --- base-commit: 5005aed8a7e728d028efb40e243ecfc2b4f3df3a change-id: 20231211-cocoa-576b8639e9bd Best regards, -- Akihiko Odaki
[PATCH v12 04/10] ui/cocoa: Scale with NSView instead of Core Graphics
Core Graphics is not accelerated and slow. Signed-off-by: Akihiko Odaki Reviewed-by: Peter Maydell --- ui/cocoa.m | 24 ++-- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index c73ef8884454..644fd32eaa4d 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -496,10 +496,8 @@ - (void) drawRect:(NSRect) rect [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 = rectList[i]; +clipRect.origin.y = (float)h - (clipRect.origin.y + clipRect.size.height); clipImageRef = CGImageCreateWithImageInRect( imageRef, clipRect @@ -545,6 +543,11 @@ - (void) setContentDimensions } } +- (void) updateBounds +{ +[self setBoundsSize:NSMakeSize(screen.width, screen.height)]; +} + - (void) updateUIInfoLocked { /* Must be called with the BQL, i.e. via updateUIInfo */ @@ -634,6 +637,7 @@ - (void) switchSurface:(pixman_image_t *)image screen.height = h; [self setContentDimensions]; [self setFrame:NSMakeRect(cx, cy, cw, ch)]; +[self updateBounds]; } // update screenBuffer @@ -1297,6 +1301,7 @@ - (void)windowDidChangeScreen:(NSNotification *)notification - (void)windowDidResize:(NSNotification *)notification { +[cocoaView updateBounds]; [cocoaView updateUIInfo]; } @@ -1945,16 +1950,7 @@ static void cocoa_update(DisplayChangeListener *dcl, COCOA_DEBUG("qemu_cocoa: cocoa_update\n"); dispatch_async(dispatch_get_main_queue(), ^{ -NSRect rect; -if ([cocoaView cdx] == 1.0) { -rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h); -} else { -rect = NSMakeRect( -x * [cocoaView cdx], -([cocoaView gscreen].height - y - h) * [cocoaView cdy], -w * [cocoaView cdx], -h * [cocoaView cdy]); -} +NSRect rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h); [cocoaView setNeedsDisplayInRect:rect]; }); } -- 2.43.2
Re: [PULL 00/47] ppc-for-9.0 queue
On Fri, 23 Feb 2024 at 15:56, Nicholas Piggin wrote: > > The following changes since commit 3d54cbf269d63ff1d500b35b2bcf4565ff8ad485: > > Merge tag 'hw-misc-20240222' of https://github.com/philmd/qemu into staging > (2024-02-22 15:44:29 +) > > are available in the Git repository at: > > https://gitlab.com/npiggin/qemu.git tags/pull-ppc-for-9.0-20240224 > > for you to fetch changes up to 4acc505d2236190efea94746e7f22e2c07bce5d6: > > target/ppc: optimise ppcemb_tlb_t flushing (2024-02-23 23:24:43 +1000) > > > Let's try this PR again. Since the previous attempt, I dropped the empty > already merged pci/raven patch. Also added several missed R-B tags. > > A bunch more testing showed up some issues with the new avocado > tests. FreeBSD artifact URL availability is marginal so dropped that > for now. ppc_hv_tests.py sometimes hangs due to a bug in console > interaction so marked it flaky so it's not gated behind the fix for > that. > > * Avocado tests for ppc64 run guests with emulated or nested hypervisor > * facilities, among other things. Update ppc64 CPU defaults to Power10. > * Add a new powernv10-rainier machine to better capture differences > between the different Power10 systems. > * Implement more device models for powernv. > * 4xx TLB flushing performance and correctness improvements. > * Correct gdb implementation to access some important SPRs. > * Misc cleanups and bug fixes. > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any user-visible changes. -- PMM
[PATCH v12 05/10] ui/cocoa: Fix pause label coordinates
A subview is positioned in the superview so the superview's frame should be used instead of one of the window to determine the coordinates. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 644fd32eaa4d..df8072479c82 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1432,8 +1432,8 @@ - (void)displayPause { /* Coordinates have to be calculated each time because the window can change its size */ int xCoord, yCoord, width, height; -xCoord = ([normalWindow frame].size.width - [pauseLabel frame].size.width)/2; -yCoord = [normalWindow frame].size.height - [pauseLabel frame].size.height - ([pauseLabel frame].size.height * .5); +xCoord = ([cocoaView frame].size.width - [pauseLabel frame].size.width)/2; +yCoord = [cocoaView frame].size.height - [pauseLabel frame].size.height - ([pauseLabel frame].size.height * .5); width = [pauseLabel frame].size.width; height = [pauseLabel frame].size.height; [pauseLabel setFrame: NSMakeRect(xCoord, yCoord, width, height)]; -- 2.43.2
[PATCH v12 07/10] ui/cocoa: Remove normalWindow
QemuCocoaView used to have fullScreenWindow but now it's gone, so we do no longer have to call the window specifically "normalWindow". Instead, refer to it with [-QemuCocoaView window]. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 33 + 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 88684af6f38b..05c91e4eed53 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -89,7 +89,6 @@ static void cocoa_switch(DisplayChangeListener *dcl, static void cocoa_refresh(DisplayChangeListener *dcl); -static NSWindow *normalWindow; static const DisplayChangeListenerOps dcl_ops = { .dpy_name = "cocoa", .dpy_gfx_update = cocoa_update, @@ -1063,9 +1062,9 @@ - (void) grabMouse COCOA_DEBUG("QemuCocoaView: grabMouse\n"); if (qemu_name) -[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - (Press " UC_CTRL_KEY " " UC_ALT_KEY " G to release Mouse)", qemu_name]]; +[[self window] setTitle:[NSString stringWithFormat:@"QEMU %s - (Press " UC_CTRL_KEY " " UC_ALT_KEY " G to release Mouse)", qemu_name]]; else -[normalWindow setTitle:@"QEMU - (Press " UC_CTRL_KEY " " UC_ALT_KEY " G to release Mouse)"]; +[[self window] setTitle:@"QEMU - (Press " UC_CTRL_KEY " " UC_ALT_KEY " G to release Mouse)"]; [self hideCursor]; CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled); isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:] @@ -1076,9 +1075,9 @@ - (void) ungrabMouse COCOA_DEBUG("QemuCocoaView: ungrabMouse\n"); if (qemu_name) -[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s", qemu_name]]; +[[self window] setTitle:[NSString stringWithFormat:@"QEMU %s", qemu_name]]; else -[normalWindow setTitle:@"QEMU"]; +[[self window] setTitle:@"QEMU"]; [self unhideCursor]; CGAssociateMouseAndMouseCursorPosition(TRUE); isMouseGrabbed = FALSE; @@ -1149,6 +1148,8 @@ - (void)adjustSpeed:(id)sender; @implementation QemuCocoaAppController - (id) init { +NSWindow *window; + COCOA_DEBUG("QemuCocoaAppController: init\n"); self = [super init]; @@ -1162,20 +1163,20 @@ - (id) init } // create a window -normalWindow = [[NSWindow alloc] initWithContentRect:[cocoaView frame] +window = [[NSWindow alloc] initWithContentRect:[cocoaView frame] styleMask:NSWindowStyleMaskTitled|NSWindowStyleMaskMiniaturizable|NSWindowStyleMaskClosable backing:NSBackingStoreBuffered defer:NO]; -if(!normalWindow) { +if(!window) { error_report("(cocoa) can't create window"); exit(1); } -[normalWindow setAcceptsMouseMovedEvents:YES]; -[normalWindow setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary]; -[normalWindow setTitle:qemu_name ? [NSString stringWithFormat:@"QEMU %s", qemu_name] : @"QEMU"]; -[normalWindow setContentView:cocoaView]; -[normalWindow makeKeyAndOrderFront:self]; -[normalWindow center]; -[normalWindow setDelegate: self]; +[window setAcceptsMouseMovedEvents:YES]; +[window setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary]; +[window setTitle:qemu_name ? [NSString stringWithFormat:@"QEMU %s", qemu_name] : @"QEMU"]; +[window setContentView:cocoaView]; +[window makeKeyAndOrderFront:self]; +[window center]; +[window setDelegate: self]; /* Used for displaying pause on the screen */ pauseLabel = [NSTextField new]; @@ -1298,7 +1299,7 @@ - (void) windowDidResignKey: (NSNotification *)aNotification */ - (void) doToggleFullScreen:(id)sender { -[normalWindow toggleFullScreen:sender]; +[[cocoaView window] toggleFullScreen:sender]; } - (void) setFullGrab:(id)sender @@ -1982,7 +1983,7 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) /* if fullscreen mode is to be used */ if (opts->has_full_screen && opts->full_screen) { -[normalWindow toggleFullScreen: nil]; +[[cocoaView window] toggleFullScreen: nil]; } if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) { [controller setFullGrab: nil]; -- 2.43.2
[PATCH v12 01/10] ui/cocoa: Split [-QemuCocoaView handleEventLocked:]
Currently [-QemuCocoaView handleEventLocked:] parses the passed event, stores operations to be done to variables, and perform them according to the variables. This construct will be cluttered with variables and hard to read when we need more different operations for different events. Split the methods so that we can call appropriate methods depending on events instead of relying on variables. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 82 +- 1 file changed, 49 insertions(+), 33 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index eb99064beeb4..32d61e226507 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1094,42 +1094,58 @@ - (bool) handleEventLocked:(NSEvent *)event } if (mouse_event) { -/* Don't send button events to the guest unless we've got a - * mouse grab or window focus. If we have neither then this event - * is the user clicking on the background window to activate and - * bring us to the front, which will be done by the sendEvent - * call below. We definitely don't want to pass that click through - * to the guest. +return [self handleMouseEvent:event buttons:buttons]; +} +return true; +} + +- (bool) handleMouseEvent:(NSEvent *)event buttons:(uint32_t)buttons +{ +/* Don't send button events to the guest unless we've got a + * mouse grab or window focus. If we have neither then this event + * is the user clicking on the background window to activate and + * bring us to the front, which will be done by the sendEvent + * call below. We definitely don't want to pass that click through + * to the guest. + */ +if ((isMouseGrabbed || [[self window] isKeyWindow]) && +(last_buttons != buttons)) { +static uint32_t bmap[INPUT_BUTTON__MAX] = { +[INPUT_BUTTON_LEFT] = MOUSE_EVENT_LBUTTON, +[INPUT_BUTTON_MIDDLE] = MOUSE_EVENT_MBUTTON, +[INPUT_BUTTON_RIGHT] = MOUSE_EVENT_RBUTTON +}; +qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons); +last_buttons = buttons; +} + +return [self handleMouseEvent:event]; +} + +- (bool) handleMouseEvent:(NSEvent *)event +{ +if (!isMouseGrabbed) { +return false; +} + +if (isAbsoluteEnabled) { +NSPoint p = [self screenLocationOfEvent:event]; + +/* Note that the origin for Cocoa mouse coords is bottom left, not top left. + * The check on screenContainsPoint is to avoid sending out of range values for + * clicks in the titlebar. */ -if ((isMouseGrabbed || [[self window] isKeyWindow]) && -(last_buttons != buttons)) { -static uint32_t bmap[INPUT_BUTTON__MAX] = { -[INPUT_BUTTON_LEFT] = MOUSE_EVENT_LBUTTON, -[INPUT_BUTTON_MIDDLE] = MOUSE_EVENT_MBUTTON, -[INPUT_BUTTON_RIGHT] = MOUSE_EVENT_RBUTTON -}; -qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons); -last_buttons = buttons; +if ([self screenContainsPoint:p]) { +qemu_input_queue_abs(dcl.con, INPUT_AXIS_X, p.x, 0, screen.width); +qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height); } -if (isMouseGrabbed) { -if (isAbsoluteEnabled) { -/* Note that the origin for Cocoa mouse coords is bottom left, not top left. - * The check on screenContainsPoint is to avoid sending out of range values for - * clicks in the titlebar. - */ -if ([self screenContainsPoint:p]) { -qemu_input_queue_abs(dcl.con, INPUT_AXIS_X, p.x, 0, screen.width); -qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height); -} -} else { -qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]); -qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]); -} -} else { -return false; -} -qemu_input_event_sync(); +} else { +qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]); +qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]); } + +qemu_input_event_sync(); + return true; } -- 2.43.2
[PATCH v12 06/10] ui/cocoa: Let the platform toggle fullscreen
It allows making the window full screen by clicking full screen button provided by the platform (the left-top green button) and save some code. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 408 +++-- 1 file changed, 181 insertions(+), 227 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index df8072479c82..88684af6f38b 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -303,20 +303,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; @@ -332,8 +329,6 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled; */ - (BOOL) isMouseGrabbed; - (BOOL) isAbsoluteEnabled; -- (float) cdx; -- (float) cdy; - (QEMUScreen) gscreen; - (void) raiseAllKeys; @end @@ -391,46 +386,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) { @@ -510,36 +502,25 @@ - (void) drawRect:(NSRect) rect } } -- (void) setContentDimensions +- (NSSize) screenSafeAreaSize { -COCOA_DEBUG("QemuCocoaView: setContentDimensions\n"); +NSSize size = [[[self window] screen] frame].size; +NSEdgeInsets insets = [[[self window] screen] safeAreaInsets]; +size.width -= insets.left + insets.right; +size.height -= insets.top + insets.bottom; +return size; +} -if (isFullscreen) { -cdx = [[NSScreen mainScreen] frame].size.width / (float)screen.width; -cdy = [[NSScreen mainScreen] frame].size.height / (float)screen.height; +- (void) resizeWindow +{ +[[self window] setContentAspectRatio:NSMakeSize(screen.width, screen.height)]; -/* stretches video, but keeps same aspect ratio */ -if (stretch_video == true) { -/* use smallest stretch value - prevents clipping on sides */ -if (MIN(cdx, cdy) == cdx) { -cdy = cdx; -} else { -cdx = cdy; -} -} else { /* No stretching */ -cdx = cdy = 1; -} -cw = screen.width * cdx; -ch = screen.height * cdy; -cx = ([[NSScreen ma
[PATCH v12 08/10] ui/cocoa: Make window resizable
The window will be resizable when zoom-to-fit is on. Signed-off-by: Akihiko Odaki Reviewed-by: Peter Maydell --- ui/cocoa.m | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/cocoa.m b/ui/cocoa.m index 05c91e4eed53..cebfae04d9e8 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -1348,8 +1348,10 @@ - (void)zoomToFit:(id) sender { stretch_video = !stretch_video; if (stretch_video == true) { +[cocoaView window].styleMask |= NSWindowStyleMaskResizable; [sender setState: NSControlStateValueOn]; } else { +[cocoaView window].styleMask &= ~NSWindowStyleMaskResizable; [cocoaView resizeWindow]; [sender setState: NSControlStateValueOff]; } @@ -2002,6 +2004,7 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) if (opts->u.cocoa.has_zoom_to_fit && opts->u.cocoa.zoom_to_fit) { stretch_video = true; +[cocoaView window].styleMask |= NSWindowStyleMaskResizable; } create_initial_menus(); -- 2.43.2
Re: [PATCH v2 1/3] qtest: migration: Enhance qtest migration functions to support 'channels' argument
On 24/02/24 1:42 am, Fabiano Rosas wrote: Het Gala writes: Introduce support for adding a 'channels' argument to migrate_qmp_fail, migrate_incoming_qmp and migrate_qmp functions within the migration qtest framework, enabling enhanced control over migration scenarios. Can't we just pass a channels string like you did in the original series with migrate_postcopy_prepare? We'd change migrate_* functions like this: void migrate_qmp(QTestState *who, const char *uri, const char *channels, const char *fmt, ...) { ... g_assert(!qdict_haskey(args, "uri")); if (uri) { qdict_put_str(args, "uri", uri); } g_assert(!qdict_haskey(args, "channels")); if (channels) { qdict_put_str(args, "channels", channels); } } Write the test like this: static void test_multifd_tcp_none_channels(void) { MigrateCommon args = { .listen_uri = "defer", .start_hook = test_migrate_precopy_tcp_multifd_start, .live = true, .connect_channels = "'channels': [ { 'channel-type': 'main'," " 'addr': { 'transport': 'socket'," "'type': 'inet'," "'host': '127.0.0.1'," "'port': '0' } } ]", .connect_uri = NULL; }; test_precopy_common(&args); } this was the same first approach that I attempted. It won't work because The final 'migrate' QAPI with channels string would look like { "execute": "migrate", "arguments": { "channels": "[ { "channel-type": "main", "addr": { "transport": "socket", "type": "inet", "host": "10.117.29.84", "port": "4000" }, "multifd-channels": 2 } ]" } } instead of { "execute": "migrate", "arguments": { "channels": [ { "channel-type": "main", "addr": { "transport": "socket", "type": "inet", "host": "10.117.29.84", "port": "4000" }, "multifd-channels": 2 } ] } } It would complain, that channels should be an *array* and not a string. So, that's the reason parsing was required in qtest too. I would be glad to hear if there are any ideas to convert /string -> json object -> add it inside qdict along with uri/ ? static void do_test_validate_uri_channel(MigrateCommon *args) { QTestState *from, *to; g_autofree char *connect_uri = NULL; if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) { return; } wait_for_serial("src_serial"); if (args->result == MIG_TEST_QMP_ERROR) { migrate_qmp_fail(from, args->connect_uri, args->connect_channels, "{}"); } else { migrate_qmp(from, args->connect_uri, args->connect_channels, "{}"); } test_migrate_end(from, to, false); } It's better to require test writers to pass in their own uri and channel strings. Otherwise any new transport added will require people to modify these conversion helpers. I agree with your point here. I was thinking to have a general but a hacky version of migrate_uri_parse() but that too seemed like a overkill. I don't have a better solution to this right now Also, using the same string as the user would use in QMP helps with development in general. One could refer to the tests to see how to invoke the migration or experiment with the string in the tests during development. For examples, I think - enough examples with 'channel' argument are provided where 'migrate' QAPI is defined. users can directly copy the qmp command from there itself. Regards, Het Gala
Re: [PATCH v11 3/6] ui/cocoa: Let the platform toggle fullscreen
On 2024/02/23 1:56, Peter Maydell wrote: On Sat, 17 Feb 2024 at 11:19, Akihiko Odaki wrote: It allows making the window full screen by clicking full screen button provided by the platform (the left-top green button) and save some code. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 423 ++--- 1 file changed, 207 insertions(+), 216 deletions(-) This is a big patch and I'm having difficulty figuring out what it's doing. Is it possible to split it up a bit? Not really. It indeed does a few different things: 1. Removes fullScreenWindow 2. Changes mouse event handling. 3. Changes coordinate calculations. Unfortunately we have to do these at once because how window is organized affects both of mouse event handling and coordinate calculations. Regards, Akihiko Odaki
Re: [PATCH v11 6/6] ui/cocoa: Remove stretch_video flag
On 2024/02/23 1:53, Peter Maydell wrote: On Sat, 17 Feb 2024 at 11:19, Akihiko Odaki wrote: Evaluate [normalWindow styleMask] & NSWindowStyleMaskResizable instead. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 81de8d92669b..401ed0c3f1f5 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -103,7 +103,6 @@ static void cocoa_switch(DisplayChangeListener *dcl, static int left_command_key_enabled = 1; static bool swap_opt_cmd; -static bool stretch_video; static NSTextField *pauseLabel; static bool allow_events; @@ -533,7 +532,7 @@ - (void) resizeWindow { [[self window] setContentAspectRatio:NSMakeSize(screen.width, screen.height)]; -if (!stretch_video) { +if (!([[self window] styleMask] & NSWindowStyleMaskResizable)) { [[self window] setContentSize:NSMakeSize(screen.width, screen.height)]; [[self window] center]; } else if ([[self window] styleMask] & NSWindowStyleMaskFullScreen) { @@ -1296,7 +1295,7 @@ - (BOOL)windowShouldClose:(id)sender - (NSSize) window:(NSWindow *)window willUseFullScreenContentSize:(NSSize)proposedSize { -if (stretch_video) { +if ([normalWindow styleMask] & NSWindowStyleMaskResizable) { return [cocoaView fixZoomedFullScreenSize:proposedSize]; } @@ -1377,8 +1376,7 @@ - (void)showQEMUDoc:(id)sender /* Stretches video to fit host monitor size */ - (void)zoomToFit:(id) sender { -stretch_video = !stretch_video; -if (stretch_video == true) { +if (([normalWindow styleMask] & NSWindowStyleMaskResizable) == 0) { [normalWindow setStyleMask:[normalWindow styleMask] | NSWindowStyleMaskResizable]; [sender setState: NSControlStateValueOn]; } else { @@ -1650,7 +1648,7 @@ static void create_initial_menus(void) menu = [[NSMenu alloc] initWithTitle:@"View"]; [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(doToggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen menuItem = [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit" action:@selector(zoomToFit:) keyEquivalent:@""] autorelease]; -[menuItem setState: stretch_video ? NSControlStateValueOn : NSControlStateValueOff]; +[menuItem setState: [normalWindow styleMask] & NSWindowStyleMaskResizable ? NSControlStateValueOn : NSControlStateValueOff]; [menu addItem: menuItem]; menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil keyEquivalent:@""] autorelease]; [menuItem setSubmenu:menu]; @@ -2036,7 +2034,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) } if (opts->u.cocoa.has_zoom_to_fit && opts->u.cocoa.zoom_to_fit) { -stretch_video = true; [normalWindow setStyleMask:[normalWindow styleMask] | NSWindowStyleMaskResizable]; } It's nice to get rid of the boolean, but [normalWindow styleMask] & NSWindowStyleMaskResizable feels a bit clunky -- maybe we should wrap it in a method with a suitable name ? (isZoomToFit, maybe? but I'm not too familiar with what cocoa function naming style is.) I don't think we should have a method for this. It's verbose but the main reason of that is NSWindowStyleMaskResizable is wordy; otherwise it's quite a simple. There are few places that use this construct after all. In any case, I put a bit more effort to simplify style mask references with v12 so please check it out. Regards, Akihiko Odaki
Re: [PATCH v11 6/6] ui/cocoa: Remove stretch_video flag
On Sat, 24 Feb 2024, Akihiko Odaki wrote: On 2024/02/23 1:53, Peter Maydell wrote: On Sat, 17 Feb 2024 at 11:19, Akihiko Odaki wrote: Evaluate [normalWindow styleMask] & NSWindowStyleMaskResizable instead. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 81de8d92669b..401ed0c3f1f5 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -103,7 +103,6 @@ static void cocoa_switch(DisplayChangeListener *dcl, static int left_command_key_enabled = 1; static bool swap_opt_cmd; -static bool stretch_video; static NSTextField *pauseLabel; static bool allow_events; @@ -533,7 +532,7 @@ - (void) resizeWindow { [[self window] setContentAspectRatio:NSMakeSize(screen.width, screen.height)]; -if (!stretch_video) { +if (!([[self window] styleMask] & NSWindowStyleMaskResizable)) { [[self window] setContentSize:NSMakeSize(screen.width, screen.height)]; [[self window] center]; } else if ([[self window] styleMask] & NSWindowStyleMaskFullScreen) { @@ -1296,7 +1295,7 @@ - (BOOL)windowShouldClose:(id)sender - (NSSize) window:(NSWindow *)window willUseFullScreenContentSize:(NSSize)proposedSize { -if (stretch_video) { +if ([normalWindow styleMask] & NSWindowStyleMaskResizable) { return [cocoaView fixZoomedFullScreenSize:proposedSize]; } @@ -1377,8 +1376,7 @@ - (void)showQEMUDoc:(id)sender /* Stretches video to fit host monitor size */ - (void)zoomToFit:(id) sender { -stretch_video = !stretch_video; -if (stretch_video == true) { +if (([normalWindow styleMask] & NSWindowStyleMaskResizable) == 0) { [normalWindow setStyleMask:[normalWindow styleMask] | NSWindowStyleMaskResizable]; [sender setState: NSControlStateValueOn]; } else { @@ -1650,7 +1648,7 @@ static void create_initial_menus(void) menu = [[NSMenu alloc] initWithTitle:@"View"]; [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(doToggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen menuItem = [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit" action:@selector(zoomToFit:) keyEquivalent:@""] autorelease]; -[menuItem setState: stretch_video ? NSControlStateValueOn : NSControlStateValueOff]; +[menuItem setState: [normalWindow styleMask] & NSWindowStyleMaskResizable ? NSControlStateValueOn : NSControlStateValueOff]; [menu addItem: menuItem]; menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil keyEquivalent:@""] autorelease]; [menuItem setSubmenu:menu]; @@ -2036,7 +2034,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) } if (opts->u.cocoa.has_zoom_to_fit && opts->u.cocoa.zoom_to_fit) { -stretch_video = true; [normalWindow setStyleMask:[normalWindow styleMask] | NSWindowStyleMaskResizable]; } It's nice to get rid of the boolean, but [normalWindow styleMask] & NSWindowStyleMaskResizable feels a bit clunky -- maybe we should wrap it in a method with a suitable name ? (isZoomToFit, maybe? but I'm not too familiar with what cocoa function naming style is.) I don't think we should have a method for this. It's verbose but the main reason of that is NSWindowStyleMaskResizable is wordy; otherwise it's quite a simple. There are few places that use this construct after all. If not a method how about a macro just to make it less long? But if Peter is otherwise happy with the current version just this does not need another version IMO. Regards, BALATON Zoltan
Re: [PATCH] hw/nvme: fix invalid endian conversion
On 24-02-22 10:29:06, Klaus Jensen wrote: > From: Klaus Jensen > > numcntl is one byte and so is max_vfs. Using cpu_to_le16 on big endian > hosts results in numcntl being set to 0. > > Fix by dropping the endian conversion. > > Fixes: 746d42b13368 ("hw/nvme: Initialize capability structures for > primary/secondary controllers") > Reported-by: Kevin Wolf > Signed-off-by: Klaus Jensen Reviewed-by: Minwoo Im Thanks,
[PATCH v2 2/6] hw/i386/pc: Rename "bus" attribute to "pcibus"
The attribute is of type PCIBus; reflect that in the name. It will also make the next change more intuitive. Suggested-by: Philippe Mathieu-Daudé Signed-off-by: Bernhard Beschow --- include/hw/i386/pc.h | 2 +- hw/i386/acpi-build.c | 2 +- hw/i386/amd_iommu.c | 2 +- hw/i386/intel_iommu.c| 2 +- hw/i386/kvm/xen_evtchn.c | 2 +- hw/i386/pc.c | 8 hw/i386/pc_piix.c| 6 +++--- hw/i386/pc_q35.c | 2 +- hw/i386/x86-iommu.c | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index e88468131a..27834043c3 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -31,7 +31,7 @@ typedef struct PCMachineState { Notifier machine_done; /* Pointers to devices and objects: */ -PCIBus *bus; +PCIBus *pcibus; I2CBus *smbus; PFlashCFI01 *flash[2]; ISADevice *pcspk; diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index d3ce96dd9f..cd3f2d0148 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1556,7 +1556,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, } crs_range_set_init(&crs_range_set); -bus = PC_MACHINE(machine)->bus; +bus = PC_MACHINE(machine)->pcibus; if (bus) { QLIST_FOREACH(bus, &bus->child, sibling) { uint8_t bus_num = pci_bus_num(bus); diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 7329553ad3..6d4fde72f9 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -1584,7 +1584,7 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) MachineState *ms = MACHINE(qdev_get_machine()); PCMachineState *pcms = PC_MACHINE(ms); X86MachineState *x86ms = X86_MACHINE(ms); -PCIBus *bus = pcms->bus; +PCIBus *bus = pcms->pcibus; s->iotlb = g_hash_table_new_full(amdvi_uint64_hash, amdvi_uint64_equal, g_free, g_free); diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index cf933189d3..cc8e59674e 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -4183,7 +4183,7 @@ static void vtd_realize(DeviceState *dev, Error **errp) MachineState *ms = MACHINE(qdev_get_machine()); PCMachineState *pcms = PC_MACHINE(ms); X86MachineState *x86ms = X86_MACHINE(ms); -PCIBus *bus = pcms->bus; +PCIBus *bus = pcms->pcibus; IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev); X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s); diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c index 0171ef6d59..a5052c0ea3 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -371,7 +371,7 @@ static int set_callback_pci_intx(XenEvtchnState *s, uint64_t param) return 0; } -pdev = pci_find_device(pcms->bus, bus, devfn); +pdev = pci_find_device(pcms->pcibus, bus, devfn); if (!pdev) { return 0; } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f8eb684a49..353edeb2ea 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -675,7 +675,7 @@ void pc_machine_done(Notifier *notifier, void *data) PCMachineState, machine_done); X86MachineState *x86ms = X86_MACHINE(pcms); -cxl_hook_up_pxb_registers(pcms->bus, &pcms->cxl_devices_state, +cxl_hook_up_pxb_registers(pcms->pcibus, &pcms->cxl_devices_state, &error_fatal); if (pcms->cxl_devices_state.is_enabled) { @@ -685,7 +685,7 @@ void pc_machine_done(Notifier *notifier, void *data) /* set the number of CPUs */ x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus); -fw_cfg_add_extra_pci_roots(pcms->bus, x86ms->fw_cfg); +fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg); acpi_setup(); if (x86ms->fw_cfg) { @@ -1250,8 +1250,8 @@ void pc_basic_device_init(struct PCMachineState *pcms, xen_evtchn_create(IOAPIC_NUM_PINS, gsi); xen_gnttab_create(); xen_xenstore_create(); -if (pcms->bus) { -pci_create_simple(pcms->bus, -1, "xen-platform"); +if (pcms->pcibus) { +pci_create_simple(pcms->pcibus, -1, "xen-platform"); } xen_bus_init(); xen_be_init(); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7724396ead..3393b93007 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -216,7 +216,7 @@ static void pc_init1(MachineState *machine, pci_bus_map_irqs(pci_bus, xen_enabled() ? xen_pci_slot_get_pirq : pc_pci_slot_get_pirq); -pcms->bus = pci_bus; +pcms->pcibus = pci_bus; hole64_size = object_property_get_uint(phb, PCI_HOST_PROP_PCI_HOLE64_SIZE, @@ -480,8 +480,8 @@ static void pc_xen_hvm_init(MachineState *machine) } pc_xen_hvm_init_pci(machine); -xen_igd_reserve_slot(pcms->bus); -pci_create_simple(pcms->bus, -1,
[PATCH v2 4/6] hw/i386/pc: Remove unneeded class attribute "kvmclock_enabled"
PCMachineClass introduces the attribute into the class hierarchy and sets it to true. There is no sub class overriding the attribute. Commit 30d2a17b46e9 "hw/i386: Remove the deprecated machines 0.12 up to 0.15" removed the last overrides of this attribute. The attribute is now unneeded and can be removed. Fixes: 30d2a17b46e9 "hw/i386: Remove the deprecated machines 0.12 up to 0.15" Cc: Thomas Huth Signed-off-by: Bernhard Beschow --- include/hw/i386/pc.h | 1 - hw/i386/pc.c | 1 - hw/i386/pc_piix.c| 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 27834043c3..4bb1899602 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -92,7 +92,6 @@ struct PCMachineClass { /* Device configuration: */ bool pci_enabled; -bool kvmclock_enabled; const char *default_south_bridge; /* Compat options: */ diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 353edeb2ea..a80f809b83 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1799,7 +1799,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc->smbios_uuid_encoded = true; pcmc->gigabyte_align = true; pcmc->has_reserved_memory = true; -pcmc->kvmclock_enabled = true; pcmc->enforce_aligned_dimm = true; pcmc->enforce_amd_1tb_hole = true; /* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K reported diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 814d24326d..49d5d48db9 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -184,7 +184,7 @@ static void pc_init1(MachineState *machine, pc_machine_init_sgx_epc(pcms); x86_cpus_init(x86ms, pcmc->default_cpu_version); -if (kvm_enabled() && pcmc->kvmclock_enabled) { +if (kvm_enabled()) { kvmclock_create(pcmc->kvmclock_create_always); } -- 2.44.0
[PATCH v2 0/6] Simplify initialization of PC machines
The series aims to simplify the initialization process of all PC-based machines by streamlining redundant code. Since I haven't seen patches on the list so far for folding CMOS data generation into pc.c, which frees all PC machines from performing this duty explicitly, I've appended this cleanup as the last two patches. Testing done: * `make check` * `make check-avocado` * I'm sending this series from within a VM containing these changes. v2: * Rebase onto master, leaving only patches 1, 3, and 5 * Patch 2: Rename "bus" attribute to "pcibus" (Phil) * Patch 4: Spotted while rebasing * Patch 6: New patch possible after [1] Best regards, Bernhard [1] https://patchew.org/QEMU/20240221211626.48190-1-phi...@linaro.org/20240221211626 .48190-10-phi...@linaro.org/ Bernhard Beschow (6): hw/i386/x86: Let ioapic_init_gsi() take parent as pointer hw/i386/pc: Rename "bus" attribute to "pcibus" hw/i386/pc_{piix,q35}: Eliminate local pci_bus/pci_host variables hw/i386/pc: Remove unneeded class attribute "kvmclock_enabled" hw/i386/pc: Populate RTC attribute directly hw/i386/pc: Inline pc_cmos_init() into pc_cmos_init_late() and remove it include/hw/i386/pc.h | 5 + include/hw/i386/x86.h| 2 +- hw/i386/acpi-build.c | 2 +- hw/i386/amd_iommu.c | 2 +- hw/i386/intel_iommu.c| 2 +- hw/i386/kvm/xen_evtchn.c | 2 +- hw/i386/microvm.c| 2 +- hw/i386/pc.c | 27 -- hw/i386/pc_piix.c| 42 +--- hw/i386/pc_q35.c | 25 ++-- hw/i386/x86-iommu.c | 2 +- hw/i386/x86.c| 7 +++ 12 files changed, 43 insertions(+), 77 deletions(-) -- 2.44.0
[PATCH v2 6/6] hw/i386/pc: Inline pc_cmos_init() into pc_cmos_init_late() and remove it
Now that pc_cmos_init() doesn't populate the X86MachineState::rtc attribute any longer, its duties can be merged into pc_cmos_init_late() which is called within machine_done notifier. This frees pc_piix and pc_q35 from explicit CMOS initialization. Signed-off-by: Bernhard Beschow --- include/hw/i386/pc.h | 2 -- hw/i386/pc.c | 10 -- hw/i386/pc_piix.c| 2 -- hw/i386/pc_q35.c | 2 -- 4 files changed, 16 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 4bb1899602..2b7c53d619 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -178,8 +178,6 @@ void pc_basic_device_init(struct PCMachineState *pcms, ISADevice *rtc_state, bool create_fdctrl, uint32_t hpet_irqs); -void pc_cmos_init(PCMachineState *pcms, - ISADevice *s); void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus); void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs); diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 880e95de26..fad4c54512 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -567,14 +567,6 @@ static void pc_cmos_init_late(PCMachineState *pcms) mc146818rtc_set_cmos_data(s, 0x39, val); pc_cmos_init_floppy(s, pc_find_fdc0()); -} - -void pc_cmos_init(PCMachineState *pcms, - ISADevice *rtc) -{ -int val; -X86MachineState *x86ms = X86_MACHINE(pcms); -MC146818RtcState *s = MC146818_RTC(rtc); /* various important CMOS locations needed by PC/Bochs bios */ @@ -617,8 +609,6 @@ void pc_cmos_init(PCMachineState *pcms, val |= 0x02; /* FPU is there */ val |= 0x04; /* PS/2 mouse installed */ mc146818rtc_set_cmos_data(s, REG_EQUIPMENT_BYTE, val); - -/* hard drives and FDC are handled by pc_cmos_init_late() */ } static void handle_a20_line_change(void *opaque, int irq, int level) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ce6aad758d..637f4d38be 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -342,8 +342,6 @@ static void pc_init1(MachineState *machine, } #endif -pc_cmos_init(pcms, x86ms->rtc); - if (piix4_pm) { smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index e0b3f55a02..df0a5f934c 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -310,8 +310,6 @@ static void pc_q35_init(MachineState *machine) smbus_eeprom_init(pcms->smbus, 8, NULL, 0); } -pc_cmos_init(pcms, x86ms->rtc); - /* the rest devices to which pci devfn is automatically assigned */ pc_vga_init(isa_bus, pcms->pcibus); pc_nic_init(pcmc, isa_bus, pcms->pcibus); -- 2.44.0
[PATCH v2 1/6] hw/i386/x86: Let ioapic_init_gsi() take parent as pointer
Rather than taking a QOM name which has to be resolved, let's pass the parent directly as pointer. This simplifies the code. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin --- include/hw/i386/x86.h | 2 +- hw/i386/microvm.c | 2 +- hw/i386/pc_piix.c | 7 +++ hw/i386/pc_q35.c | 2 +- hw/i386/x86.c | 7 +++ 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index 8e306db7bb..4dc30dcb4d 100644 --- a/include/hw/i386/x86.h +++ b/include/hw/i386/x86.h @@ -139,7 +139,7 @@ typedef struct GSIState { qemu_irq x86_allocate_cpu_irq(void); void gsi_handler(void *opaque, int n, int level); -void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name); +void ioapic_init_gsi(GSIState *gsi_state, Object *parent); DeviceState *ioapic_init_secondary(GSIState *gsi_state); /* pc_sysfw.c */ diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c index ca55aecc3b..61a772dfe6 100644 --- a/hw/i386/microvm.c +++ b/hw/i386/microvm.c @@ -175,7 +175,7 @@ static void microvm_devices_init(MicrovmMachineState *mms) &error_abort); isa_bus_register_input_irqs(isa_bus, x86ms->gsi); -ioapic_init_gsi(gsi_state, "machine"); +ioapic_init_gsi(gsi_state, OBJECT(mms)); if (ioapics > 1) { x86ms->ioapic2 = ioapic_init_secondary(gsi_state); } diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ec7c07b362..7724396ead 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -107,6 +107,7 @@ static void pc_init1(MachineState *machine, X86MachineState *x86ms = X86_MACHINE(machine); MemoryRegion *system_memory = get_system_memory(); MemoryRegion *system_io = get_system_io(); +Object *phb = NULL; PCIBus *pci_bus = NULL; ISABus *isa_bus; Object *piix4_pm = NULL; @@ -189,8 +190,6 @@ static void pc_init1(MachineState *machine, } if (pcmc->pci_enabled) { -Object *phb; - pci_memory = g_new(MemoryRegion, 1); memory_region_init(pci_memory, NULL, "pci", UINT64_MAX); rom_memory = pci_memory; @@ -303,8 +302,8 @@ static void pc_init1(MachineState *machine, pc_i8259_create(isa_bus, gsi_state->i8259_irq); } -if (pcmc->pci_enabled) { -ioapic_init_gsi(gsi_state, "i440fx"); +if (phb) { +ioapic_init_gsi(gsi_state, phb); } if (tcg_enabled()) { diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 53fb3db26d..c89ff63579 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -263,7 +263,7 @@ static void pc_q35_init(MachineState *machine) pc_i8259_create(isa_bus, gsi_state->i8259_irq); } -ioapic_init_gsi(gsi_state, "q35"); +ioapic_init_gsi(gsi_state, OBJECT(phb)); if (tcg_enabled()) { x86_register_ferr_irq(x86ms->gsi[13]); diff --git a/hw/i386/x86.c b/hw/i386/x86.c index 684dce90e9..807e09bcdb 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -640,20 +640,19 @@ void gsi_handler(void *opaque, int n, int level) } } -void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name) +void ioapic_init_gsi(GSIState *gsi_state, Object *parent) { DeviceState *dev; SysBusDevice *d; unsigned int i; -assert(parent_name); +assert(parent); if (kvm_ioapic_in_kernel()) { dev = qdev_new(TYPE_KVM_IOAPIC); } else { dev = qdev_new(TYPE_IOAPIC); } -object_property_add_child(object_resolve_path(parent_name, NULL), - "ioapic", OBJECT(dev)); +object_property_add_child(parent, "ioapic", OBJECT(dev)); d = SYS_BUS_DEVICE(dev); sysbus_realize_and_unref(d, &error_fatal); sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS); -- 2.44.0
[PATCH v2 5/6] hw/i386/pc: Populate RTC attribute directly
Both the piix and the q35 machines introduce an rtc_state variable and defer the initialization of the X86MachineState::rtc attribute to pc_cmos_init(). Resolve this complication which makes pc_cmos_init() do what it says on the tin. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin --- hw/i386/pc.c | 8 hw/i386/pc_piix.c | 15 +++ hw/i386/pc_q35.c | 7 +++ 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a80f809b83..880e95de26 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -611,14 +611,6 @@ void pc_cmos_init(PCMachineState *pcms, mc146818rtc_set_cmos_data(s, 0x5c, val >> 8); mc146818rtc_set_cmos_data(s, 0x5d, val >> 16); -object_property_add_link(OBJECT(pcms), "rtc_state", - TYPE_ISA_DEVICE, - (Object **)&x86ms->rtc, - object_property_allow_set_link, - OBJ_PROP_LINK_STRONG); -object_property_set_link(OBJECT(pcms), "rtc_state", OBJECT(s), - &error_abort); - set_boot_dev(s, MACHINE(pcms)->boot_config.order, &error_fatal); val = 0; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 49d5d48db9..ce6aad758d 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -112,7 +112,6 @@ static void pc_init1(MachineState *machine, Object *piix4_pm = NULL; qemu_irq smi_irq; GSIState *gsi_state; -ISADevice *rtc_state; MemoryRegion *ram_memory; MemoryRegion *pci_memory = NULL; MemoryRegion *rom_memory = system_memory; @@ -276,8 +275,8 @@ static void pc_init1(MachineState *machine, } isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0")); -rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev), - "rtc")); +x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev), + "rtc")); piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm"); dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide")); pci_ide_create_devs(PCI_DEVICE(dev)); @@ -288,9 +287,9 @@ static void pc_init1(MachineState *machine, &error_abort); isa_bus_register_input_irqs(isa_bus, x86ms->gsi); -rtc_state = isa_new(TYPE_MC146818_RTC); -qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000); -isa_realize_and_unref(rtc_state, isa_bus, &error_fatal); +x86ms->rtc = isa_new(TYPE_MC146818_RTC); +qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000); +isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal); i8257_dma_init(OBJECT(machine), isa_bus, 0); pcms->hpet_enabled = false; @@ -316,7 +315,7 @@ static void pc_init1(MachineState *machine, } /* init basic PC hardware */ -pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, true, +pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc, true, 0x4); pc_nic_init(pcmc, isa_bus, pcms->pcibus); @@ -343,7 +342,7 @@ static void pc_init1(MachineState *machine, } #endif -pc_cmos_init(pcms, rtc_state); +pc_cmos_init(pcms, x86ms->rtc); if (piix4_pm) { smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 2fa4efb52f..e0b3f55a02 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -124,7 +124,6 @@ static void pc_q35_init(MachineState *machine) Object *phb; PCIDevice *lpc; DeviceState *lpc_dev; -ISADevice *rtc_state; MemoryRegion *system_memory = get_system_memory(); MemoryRegion *system_io = get_system_io(); MemoryRegion *pci_memory = g_new(MemoryRegion, 1); @@ -231,7 +230,7 @@ static void pc_q35_init(MachineState *machine) } pci_realize_and_unref(lpc, pcms->pcibus, &error_fatal); -rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc")); +x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc")); object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP, TYPE_HOTPLUG_HANDLER, @@ -273,7 +272,7 @@ static void pc_q35_init(MachineState *machine) } /* init basic PC hardware */ -pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, !mc->no_floppy, +pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc, !mc->no_floppy, 0xff0104); if (pcms->sata_enabled) { @@ -311,7 +310,7 @@ static void pc_q35_init(MachineState *machine) smbus_eeprom_init(pcms->smbus, 8, NULL, 0); } -pc_cmos_init(pcms, rtc_state); +pc_cmos_init(pcms, x86ms->rtc); /* the rest devices to which pci devfn is automatically assigned
[PATCH v2 3/6] hw/i386/pc_{piix, q35}: Eliminate local pci_bus/pci_host variables
There is no advantage in having these local variables which 1/ needlessly have different identifiers in both machines and 2/ which are redundant to pcms->bus which is almost as short. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin --- hw/i386/pc_piix.c | 14 ++ hw/i386/pc_q35.c | 16 +++- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 3393b93007..814d24326d 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -108,7 +108,6 @@ static void pc_init1(MachineState *machine, MemoryRegion *system_memory = get_system_memory(); MemoryRegion *system_io = get_system_io(); Object *phb = NULL; -PCIBus *pci_bus = NULL; ISABus *isa_bus; Object *piix4_pm = NULL; qemu_irq smi_irq; @@ -212,11 +211,10 @@ static void pc_init1(MachineState *machine, &error_fatal); sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal); -pci_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0")); -pci_bus_map_irqs(pci_bus, +pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0")); +pci_bus_map_irqs(pcms->pcibus, xen_enabled() ? xen_pci_slot_get_pirq : pc_pci_slot_get_pirq); -pcms->pcibus = pci_bus; hole64_size = object_property_get_uint(phb, PCI_HOST_PROP_PCI_HOLE64_SIZE, @@ -261,7 +259,7 @@ static void pc_init1(MachineState *machine, for (i = 0; i < ISA_NUM_IRQS; i++) { qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]); } -pci_realize_and_unref(pci_dev, pci_bus, &error_fatal); +pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal); if (xen_enabled()) { pci_device_set_intx_routing_notifier( @@ -273,7 +271,7 @@ static void pc_init1(MachineState *machine, * connected to the IOAPIC directly. * These additional routes can be discovered through ACPI. */ -pci_bus_irqs(pci_bus, xen_intx_set_irq, pci_dev, +pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev, XEN_IOAPIC_NUM_PIRQS); } @@ -310,7 +308,7 @@ static void pc_init1(MachineState *machine, x86_register_ferr_irq(x86ms->gsi[13]); } -pc_vga_init(isa_bus, pcmc->pci_enabled ? pci_bus : NULL); +pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL); assert(pcms->vmport != ON_OFF_AUTO__MAX); if (pcms->vmport == ON_OFF_AUTO_AUTO) { @@ -321,7 +319,7 @@ static void pc_init1(MachineState *machine, pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, true, 0x4); -pc_nic_init(pcmc, isa_bus, pci_bus); +pc_nic_init(pcmc, isa_bus, pcms->pcibus); #ifdef CONFIG_IDE_ISA if (!pcmc->pci_enabled) { diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 734d9bedb2..2fa4efb52f 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -122,7 +122,6 @@ static void pc_q35_init(MachineState *machine) PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); X86MachineState *x86ms = X86_MACHINE(machine); Object *phb; -PCIBus *host_bus; PCIDevice *lpc; DeviceState *lpc_dev; ISADevice *rtc_state; @@ -216,8 +215,7 @@ static void pc_q35_init(MachineState *machine) sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal); /* pci */ -host_bus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pcie.0")); -pcms->pcibus = host_bus; +pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pcie.0")); /* irq lines */ gsi_state = pc_gsi_create(&x86ms->gsi, true); @@ -231,7 +229,7 @@ static void pc_q35_init(MachineState *machine) for (i = 0; i < IOAPIC_NUM_PINS; i++) { qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]); } -pci_realize_and_unref(lpc, host_bus, &error_fatal); +pci_realize_and_unref(lpc, pcms->pcibus, &error_fatal); rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc")); @@ -283,7 +281,7 @@ static void pc_q35_init(MachineState *machine) AHCIPCIState *ich9; /* ahci and SATA device, for q35 1 ahci controller is built-in */ -pdev = pci_create_simple_multifunction(host_bus, +pdev = pci_create_simple_multifunction(pcms->pcibus, PCI_DEVFN(ICH9_SATA1_DEV, ICH9_SATA1_FUNC), "ich9-ahci"); @@ -297,14 +295,14 @@ static void pc_q35_init(MachineState *machine) if (machine_usb(machine)) { /* Should we create 6 UHCI according to ich9 spec? */ -ehci_create_ich9_with_companions(host_bus, 0x1d); +ehci_create_ich9_with_companions(pcms->pcibus, 0x1d);
Re: [PATCH 0/9] Simplify initialization of PC machines
Am 22. Februar 2024 15:25:01 UTC schrieb "Michael S. Tsirkin" : >On Thu, Feb 08, 2024 at 11:03:40PM +0100, Bernhard Beschow wrote: >> The series aims to simplify the initialization process of all PC-based >> machines. >> >> It consists of streamlining redundant code, as well as consolidating the >> setup >> of system flash and generation of smbios data which are currently fairly >> distributed. >> >> These changes are expected to make the code easier to understand and >> maintain. >> >> Best regards, >> Bernhard > > >This looks good to me overall. > >Reviewed-by: Michael S. Tsirkin > >I see Philippe started queueing these, fine by me. Thanks so far to all involved! I've just sent v2: https://lore.kernel.org/qemu-devel/20240224135851.100361-1-shen...@gmail.com/ Best regars, Bernhard > >> Bernhard Beschow (9): >> hw/i386/x86: Let ioapic_init_gsi() take parent as pointer >> hw/i386/pc_piix: Share pc_cmos_init() invocation between pc and isapc >> machines >> hw/i386/x86: Turn apic_xrupt_override into class attribute >> hw/i386/pc: Merge pc_guest_info_init() into pc_machine_initfn() >> hw/i386/pc: Defer smbios_set_defaults() to machine_done >> hw/i386/pc: Confine system flash handling to pc_sysfw >> hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove it >> hw/i386/pc: Populate RTC attribute directly >> hw/i386/pc_{piix,q35}: Eliminate local pci_bus/pci_host variables >> >> hw/i386/fw_cfg.h | 3 ++- >> include/hw/i386/pc.h | 5 >> include/hw/i386/x86.h | 5 ++-- >> hw/i386/acpi-common.c | 3 ++- >> hw/i386/fw_cfg.c | 12 +- >> hw/i386/microvm.c | 2 +- >> hw/i386/pc.c | 25 +--- >> hw/i386/pc_piix.c | 55 ++- >> hw/i386/pc_q35.c | 38 ++ >> hw/i386/pc_sysfw.c| 17 - >> hw/i386/x86.c | 7 +++--- >> 11 files changed, 62 insertions(+), 110 deletions(-) >> >> -- >> 2.43.0 >> >
[PATCH v2] ui/cocoa: Fix window clipping on macOS 14
macOS Sonoma changes the NSView.clipsToBounds to false by default where it was true in earlier version of macOS. This causes the window contents to be occluded by the frame at the top of the window. This fixes the issue by conditionally compiling the clipping on Sonoma to true. NSView only exposes the clipToBounds in macOS 14 and so has to be fixed via conditional compilation. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1994 Signed-off-by: David Parsons --- ui/cocoa.m | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ui/cocoa.m b/ui/cocoa.m index eb99064bee..bbf9704b8c 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -54,6 +54,10 @@ #define MAC_OS_X_VERSION_10_13 101300 #endif +#ifndef MAC_OS_VERSION_14_0 +#define MAC_OS_VERSION_14_0 14 +#endif + /* 10.14 deprecates NSOnState and NSOffState in favor of * NSControlStateValueOn/Off, which were introduced in 10.13. * Define for older versions @@ -365,6 +369,9 @@ - (id)initWithFrame:(NSRect)frameRect screen.width = frameRect.size.width; screen.height = frameRect.size.height; kbd = qkbd_state_init(dcl.con); +#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_14_0 +[self setClipsToBounds:YES]; +#endif } return self; -- 2.39.3 (Apple Git-145)
Re: [PATCH v2 0/6] Simplify initialization of PC machines
Am 24. Februar 2024 13:58:45 UTC schrieb Bernhard Beschow : >The series aims to simplify the initialization process of all PC-based machines > >by streamlining redundant code. > > > >Since I haven't seen patches on the list so far for folding CMOS data > >generation into pc.c, which frees all PC machines from performing this duty > >explicitly, I've appended this cleanup as the last two patches. > > > >Testing done: > >* `make check` > The `boot-order-test` actually fails. We'd have to ignore the last patch for now. Best regards, Bernhard >* `make check-avocado` > >* I'm sending this series from within a VM containing these changes. > > > >v2: > >* Rebase onto master, leaving only patches 1, 3, and 5 > >* Patch 2: Rename "bus" attribute to "pcibus" (Phil) > >* Patch 4: Spotted while rebasing > >* Patch 6: New patch possible after [1] > > > >Best regards, > >Bernhard > > > >[1] > >https://patchew.org/QEMU/20240221211626.48190-1-phi...@linaro.org/20240221211626 > >.48190-10-phi...@linaro.org/ > > > >Bernhard Beschow (6): > > hw/i386/x86: Let ioapic_init_gsi() take parent as pointer > > hw/i386/pc: Rename "bus" attribute to "pcibus" > > hw/i386/pc_{piix,q35}: Eliminate local pci_bus/pci_host variables > > hw/i386/pc: Remove unneeded class attribute "kvmclock_enabled" > > hw/i386/pc: Populate RTC attribute directly > > hw/i386/pc: Inline pc_cmos_init() into pc_cmos_init_late() and remove > >it > > > > include/hw/i386/pc.h | 5 + > > include/hw/i386/x86.h| 2 +- > > hw/i386/acpi-build.c | 2 +- > > hw/i386/amd_iommu.c | 2 +- > > hw/i386/intel_iommu.c| 2 +- > > hw/i386/kvm/xen_evtchn.c | 2 +- > > hw/i386/microvm.c| 2 +- > > hw/i386/pc.c | 27 -- > > hw/i386/pc_piix.c| 42 +--- > > hw/i386/pc_q35.c | 25 ++-- > > hw/i386/x86-iommu.c | 2 +- > > hw/i386/x86.c| 7 +++ > > 12 files changed, 43 insertions(+), 77 deletions(-) > > > >-- > >2.44.0 > > >
Re: [PATCH v2] ui/cocoa: Fix window clipping on macOS 14
On 2024/02/24 23:06, David Parsons wrote: macOS Sonoma changes the NSView.clipsToBounds to false by default where it was true in earlier version of macOS. This causes the window contents to be occluded by the frame at the top of the window. This fixes the issue by conditionally compiling the clipping on Sonoma to true. NSView only exposes the clipToBounds in macOS 14 and so has to be fixed via conditional compilation. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1994 Signed-off-by: David Parsons Reviewed-by: Akihiko Odaki --- ui/cocoa.m | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ui/cocoa.m b/ui/cocoa.m index eb99064bee..bbf9704b8c 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -54,6 +54,10 @@ #define MAC_OS_X_VERSION_10_13 101300 #endif +#ifndef MAC_OS_VERSION_14_0 +#define MAC_OS_VERSION_14_0 14 +#endif + /* 10.14 deprecates NSOnState and NSOffState in favor of * NSControlStateValueOn/Off, which were introduced in 10.13. * Define for older versions @@ -365,6 +369,9 @@ - (id)initWithFrame:(NSRect)frameRect screen.width = frameRect.size.width; screen.height = frameRect.size.height; kbd = qkbd_state_init(dcl.con); +#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_14_0 +[self setClipsToBounds:YES]; +#endif } return self;
[PATCH] target/riscv: Fix shift count overflow
The result of (8 - 3 - vlmul) is negtive when vlmul >= 6, and results in wrong vill. Signed-off-by: demin.han --- target/riscv/vector_helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index 84cec73eb2..ced0aca633 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -53,10 +53,9 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1, * VLEN * LMUL >= SEW * VLEN >> (8 - lmul) >= sew * (vlenb << 3) >> (8 - lmul) >= sew - * vlenb >> (8 - 3 - lmul) >= sew */ if (vlmul == 4 || -cpu->cfg.vlenb >> (8 - 3 - vlmul) < sew) { +(cpu->cfg.vlenb << 3) >> (8 - vlmul) < sew) { vill = true; } } -- 2.43.2
[PATCH RFC 2/3] meson.build: Support GM/T 0018-2012 cryptographic standard
GM/T 0018-2012 is a cryptographic standard issued by the State Cryptography Administration of China. The implement of the standard could support symmetric cipher algorithm for block encryption. SM4 cipher algorithms could be applied currently, so detect SM4 cipher algorithms via GM/T 0018-2012 API and enable the feature if crypto-gmt is given explictly. This feature defaults to disabled. Signed-off-by: Hyman Huang --- crypto/meson.build| 3 +++ meson.build | 30 ++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 3 +++ 4 files changed, 38 insertions(+) diff --git a/crypto/meson.build b/crypto/meson.build index c46f9c22a7..dd49d03780 100644 --- a/crypto/meson.build +++ b/crypto/meson.build @@ -46,6 +46,9 @@ endif if have_afalg crypto_ss.add(if_true: files('afalg.c', 'cipher-afalg.c', 'hash-afalg.c')) endif +if gmt_0018_2012.found() + crypto_ss.add(gmt_0018_2012, files('cipher-gmt.c')) +endif system_ss.add(when: gnutls, if_true: files('tls-cipher-suites.c')) diff --git a/meson.build b/meson.build index c1dc83e4c0..cd188582b5 100644 --- a/meson.build +++ b/meson.build @@ -1693,6 +1693,34 @@ if not gnutls_crypto.found() endif endif +if get_option('crypto_gmt').enabled() and get_option('crypto_afalg').enabled() + error('Only one of GM/T 0018-2012 & afalg can be enabled') +endif + +gmt_0018_2012 = not_found +if (not get_option('crypto_gmt').auto() or have_system) + gmt_0018_2012 = cc.find_library('gmt_0018_2012', has_headers: ['gmt-0018-2012.h'], + required: get_option('crypto_gmt')) + if gmt_0018_2012.found() and not cc.links(''' +#include +#include +int main(void) { + unsigned char iv[16] = {0}; + unsigned char plainData[16] = {0}; + unsigned char cipherData[16] = {0}; + unsigned int rlen; + SDF_Encrypt(NULL, NULL, SGD_SM4_ECB, iv, plainData, 16, cipherData, &rlen); + return 0; +}''', dependencies: gmt_0018_2012) +gmt_0018_2012 = not_found +if get_option('crypto_gmt').enabled() + error('could not link gmt_0018_2012') +else + warning('could not link gmt_0018_2012, disabling') +endif + endif +endif + capstone = not_found if not get_option('capstone').auto() or have_system or have_user capstone = dependency('capstone', version: '>=3.0.5', @@ -2291,6 +2319,7 @@ config_host_data.set('CONFIG_GNUTLS_CRYPTO', gnutls_crypto.found()) config_host_data.set('CONFIG_TASN1', tasn1.found()) config_host_data.set('CONFIG_GCRYPT', gcrypt.found()) config_host_data.set('CONFIG_NETTLE', nettle.found()) +config_host_data.set('CONFIG_GMT_0018_2012', gmt_0018_2012.found()) config_host_data.set('CONFIG_CRYPTO_SM4', crypto_sm4.found()) config_host_data.set('CONFIG_HOGWEED', hogweed.found()) config_host_data.set('CONFIG_QEMU_PRIVATE_XTS', xts == 'private') @@ -4333,6 +4362,7 @@ if nettle.found() endif summary_info += {'SM4 ALG support': crypto_sm4} summary_info += {'AF_ALG support':have_afalg} +summary_info += {'GM/T 0018-2012 support': gmt_0018_2012.found()} summary_info += {'rng-none': get_option('rng_none')} summary_info += {'Linux keyring': have_keyring} summary_info += {'Linux keyutils':keyutils} diff --git a/meson_options.txt b/meson_options.txt index 0a99a059ec..4f35d3d62d 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -174,6 +174,8 @@ option('gcrypt', type : 'feature', value : 'auto', description: 'libgcrypt cryptography support') option('crypto_afalg', type : 'feature', value : 'disabled', description: 'Linux AF_ALG crypto backend driver') +option('crypto_gmt', type : 'feature', value : 'disabled', + description: 'GM/T 0018-2012 cryptographic standard driver') option('libdaxctl', type : 'feature', value : 'auto', description: 'libdaxctl support') option('libpmem', type : 'feature', value : 'auto', diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 680fa3f581..e116e7b9ed 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -106,6 +106,7 @@ meson_options_help() { printf "%s\n" ' colo-proxy colo-proxy support' printf "%s\n" ' coreaudio CoreAudio sound support' printf "%s\n" ' crypto-afalgLinux AF_ALG crypto backend driver' + printf "%s\n" ' crypto-gmt GM/T 0018-2012 crypto backend driver' printf "%s\n" ' curlCURL block device driver' printf "%s\n" ' curses curses UI' printf "%s\n" ' dbus-display-display dbus support' @@ -282,6 +283,8 @@ _meson_option_parse() { --disable-coroutine-pool) printf "%s" -Dcoroutine_pool=false ;; --enable-crypto-afalg) printf "%s" -Dcrypto_afalg=enabled ;; --disable-crypto-afalg) printf "%s" -Dcrypto_afalg=disabled ;; +--enable-crypto-gmt) printf "%s" -Dcrypto_gmt=enabled ;; +--disable-crypto-gmt) printf "%s" -Dcrypto_gmt=disabled ;; --enable-curl) prin
[PATCH RFC 3/3] crypto: Allow GM/T 0018-2012 to support SM4 cipher algorithm
Since GM/T 0018-2012 was probed by SM4 cipher algorithm, allow it to support SM4 cipher algorithm in block encryption. Signed-off-by: Hyman Huang --- crypto/block-luks.c | 4 ++-- crypto/cipher.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 3ee928fb5a..f4101fd435 100644 --- a/crypto/block-luks.c +++ b/crypto/block-luks.c @@ -95,7 +95,7 @@ qcrypto_block_luks_cipher_size_map_twofish[] = { { 0, 0 }, }; -#ifdef CONFIG_CRYPTO_SM4 +#if defined CONFIG_CRYPTO_SM4 || defined CONFIG_GMT_0018_2012 static const QCryptoBlockLUKSCipherSizeMap qcrypto_block_luks_cipher_size_map_sm4[] = { { 16, QCRYPTO_CIPHER_ALG_SM4}, @@ -109,7 +109,7 @@ qcrypto_block_luks_cipher_name_map[] = { { "cast5", qcrypto_block_luks_cipher_size_map_cast5 }, { "serpent", qcrypto_block_luks_cipher_size_map_serpent }, { "twofish", qcrypto_block_luks_cipher_size_map_twofish }, -#ifdef CONFIG_CRYPTO_SM4 +#if defined CONFIG_CRYPTO_SM4 || defined CONFIG_GMT_0018_2012 { "sm4", qcrypto_block_luks_cipher_size_map_sm4}, #endif }; diff --git a/crypto/cipher.c b/crypto/cipher.c index 785f231948..5c2a620dcf 100644 --- a/crypto/cipher.c +++ b/crypto/cipher.c @@ -38,7 +38,7 @@ static const size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = { [QCRYPTO_CIPHER_ALG_TWOFISH_128] = 16, [QCRYPTO_CIPHER_ALG_TWOFISH_192] = 24, [QCRYPTO_CIPHER_ALG_TWOFISH_256] = 32, -#ifdef CONFIG_CRYPTO_SM4 +#if defined CONFIG_CRYPTO_SM4 || defined CONFIG_GMT_0018_2012 [QCRYPTO_CIPHER_ALG_SM4] = 16, #endif }; @@ -56,7 +56,7 @@ static const size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = { [QCRYPTO_CIPHER_ALG_TWOFISH_128] = 16, [QCRYPTO_CIPHER_ALG_TWOFISH_192] = 16, [QCRYPTO_CIPHER_ALG_TWOFISH_256] = 16, -#ifdef CONFIG_CRYPTO_SM4 +#if defined CONFIG_CRYPTO_SM4 || defined CONFIG_GMT_0018_2012 [QCRYPTO_CIPHER_ALG_SM4] = 16, #endif }; -- 2.39.3
[PATCH RFC 1/3] crypto: Introduce GM/T 0018-2012 cryptographic driver
GM/T 0018-2012 is a cryptographic standard issued by the State Cryptography Administration of China. For more information about the standard, visit https://hbba.sacinfo.org.cn. The objective of the standard is to develop a uniform application interface standard for the service-based cryptography device under the public key cryptographic infrastructure application framework, and to call the cryptography device through this interface to provide basic cryptographic services for the uppler layer. For more information about contents of the standard, download the specificaiton from: "https://github.com/guanzhi/GM-Standards/blob/master/GMT密码行标/ GMT%200018-2012%20密码设备应用接口规范.pdf" This patch implement the basic functions of GM/T 0018-2012 standard. Currently, for block encryption, it support SM4 cipher algorithm only. Signed-off-by: Hyman Huang --- MAINTAINERS | 3 +- crypto/cipher-gmt.c | 263 crypto/cipher.c | 2 + crypto/cipherpriv.h | 6 + 4 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 crypto/cipher-gmt.c diff --git a/MAINTAINERS b/MAINTAINERS index a24c2b51b6..822726e9da 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3418,10 +3418,11 @@ F: migration/dirtyrate.c F: migration/dirtyrate.h F: include/sysemu/dirtyrate.h -Detached LUKS header +Detached LUKS header and GM/T 0018-2012 cryptography M: Hyman Huang S: Maintained F: tests/qemu-iotests/tests/luks-detached-header +F: crypto/cipher-gmt.c D-Bus M: Marc-André Lureau diff --git a/crypto/cipher-gmt.c b/crypto/cipher-gmt.c new file mode 100644 index 00..40e32c114f --- /dev/null +++ b/crypto/cipher-gmt.c @@ -0,0 +1,263 @@ +/* + * QEMU GM/T 0018-2012 cryptographic standard support + * + * Copyright (c) 2024 SmartX Inc + * + * Authors: + *Hyman Huang + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * (at your option) any later version. See the COPYING file in the + * top-level directory. + */ +#include + +#include "qemu/osdep.h" +#include "qemu/thread.h" +#include "qapi/error.h" +#include "crypto/cipher.h" +#include "cipherpriv.h" + +#include "qemu/error-report.h" + +typedef struct QCryptoGMT QCryptoGMT; + +struct QCryptoGMT { +QCryptoCipher base; + +SGD_HANDLE session; +SGD_HANDLE key; +SGD_UINT32 alg; +unsigned char iv[16]; /* not used for SM4 algo currently */ +}; + +typedef struct QCryptoGMTDeviceInfo QCryptoGMTDeviceInfo; + +struct QCryptoGMTDeviceInfo { +SGD_HANDLE device; +struct DeviceInfo_st info; +bool opened; +gint ref_count; +}; +/* + * It is advised to use numerous sessions with one open device + * as opposed to single sessions with several devices. + */ +static QCryptoGMTDeviceInfo gmt_device; +/* Protect the gmt_device */ +static QemuMutex gmt_device_mutex; + +static const struct QCryptoCipherDriver qcrypto_cipher_gmt_driver; + +static void gmt_device_lock(void) +{ +qemu_mutex_lock(&gmt_device_mutex); +} + +static void gmt_device_unlock(void) +{ +qemu_mutex_unlock(&gmt_device_mutex); +} + +static void +__attribute__((__constructor__)) gmt_device_mutex_init(void) +{ +qemu_mutex_init(&gmt_device_mutex); +} + +static void +gmt_device_ref(void) +{ +g_assert(gmt_device.device != NULL); +g_atomic_int_inc(&gmt_device.ref_count); +} + +static void +gmt_device_unref(void) +{ +g_assert(gmt_device.device != NULL); +if (g_atomic_int_dec_and_test(&gmt_device.ref_count)) { +SDF_CloseDevice(gmt_device.device); +gmt_device.opened = false; +gmt_device.device = NULL; +memset(&gmt_device.info, 0, sizeof(struct DeviceInfo_st)); +} +} + +static bool +qcrypto_gmt_cipher_supports(QCryptoCipherAlgorithm alg, +QCryptoCipherMode mode) +{ +switch (alg) { +case QCRYPTO_CIPHER_ALG_SM4: +break; +default: +return false; +} + +switch (mode) { +case QCRYPTO_CIPHER_MODE_ECB: +return true; +default: +return false; +} +} + +QCryptoCipher * +qcrypto_gmt_cipher_ctx_new(QCryptoCipherAlgorithm alg, + QCryptoCipherMode mode, + const uint8_t *key, + size_t nkey, + Error **errp) +{ +QCryptoGMT *gmt; +int rv; + +if (!qcrypto_gmt_cipher_supports(alg, mode)) { +return NULL; +} + +gmt = g_new0(QCryptoGMT, 1); +if (!gmt) { +return NULL; +} + +switch (alg) { +case QCRYPTO_CIPHER_ALG_SM4: +gmt->alg = SGD_SM4_ECB; +break; +default: +return NULL; +} + +gmt_device_lock(); +if (!gmt_device.opened) { +rv = SDF_OpenDevice(&gmt_device.device); +if (rv != SDR_OK) { +info_report("Could not open encryption card device, disabling"); +goto abort; +} +gmt_device.opened = true; +} + +/* + * multi-sessions correspond to an
[PATCH RFC 0/3] Support GM/T 0018-2012 cryptographic standard
This patchset introduce GM/T 0018-2012 as a crypto backend driver, which is applied for block encryption. Currently, we support SM4 cipher algorithm only. GM/T 0018-2012 is a cryptographic standard issued by the State Cryptography Administration of China. Visit https://hbba.sacinfo.org.cn search GM/T 0018-2012 for brief introduction. The objective of the standard is to develop a uniform application interface standard for the service-based cryptography device under the public key cryptographic infrastructure application framework, and to call the cryptography device through this interface to provide basic cryptographic services for the uppler layer. For more information about contents of the standard, download the specificaiton from: "https://github.com/guanzhi/GM-Standards/blob/master/GMT密码行标/ GMT 00018-2012 密码设备应用接口规范.pdf" There are two benefits to doing this, at least. * Performance - using a cryptography device for block encryption offers an opportunity to enhance the input/output performance once the hardware is certified * Secrecy - hardware manufacturers may fortify cryptography equipment with security features, so increasing the secrecy of block encryption. The precise way that vendors implement the standard APIs for data encryption using the cryptographic device is uncoupled from the GM/T 0018-2012 specification. Thus, if developers enable this functionality with the following conditions met, we could accomplish the general implementation: 1. rename the header file provided by vendor to gmt-0018-2012.h and copy it to the /usr/include directory. 2. rename the dynamic library provided by vendor to gmt_0018_2012.so and copy it to the /usr/lib64 or any directory that linker could find before compiling QEMU. 3. enable crypto_gmt option when compiling QEMU and make the feature availiable. By offering a development package for GM/T 0018-2012, the above provisions could be standardized; unfortunately, the hardware manufacturer has not completed this task. So developers who don't work with the vendor to obtain the cryptography device and related library may not be able to test this functionality because the standard implementation depends on the cryptography device supplied by the hardware vendor. We are hesitant to contribute to this series as a result. After all, we uploaded this series with the intention of receiving feedback, as the title suggests. We would welcome any suggestions and feedback regarding this feature. Hyman Huang (3): crypto: Introduce GM/T 0018-2012 cryptographic driver meson.build: Support GM/T 0018-2012 cryptographic standard crypto: Allow GM/T 0018-2012 to support SM4 cipher algorithm MAINTAINERS | 3 +- crypto/block-luks.c | 4 +- crypto/cipher-gmt.c | 263 ++ crypto/cipher.c | 6 +- crypto/cipherpriv.h | 6 + crypto/meson.build| 3 + meson.build | 30 meson_options.txt | 2 + scripts/meson-buildoptions.sh | 3 + 9 files changed, 315 insertions(+), 5 deletions(-) create mode 100644 crypto/cipher-gmt.c -- 2.39.3