Re: [PATCH v2 1/3] qtest: migration: Enhance qtest migration functions to support 'channels' argument

2024-02-24 Thread Het Gala


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

2024-02-24 Thread Peter Maydell
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

2024-02-24 Thread Michael Tokarev

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

2024-02-24 Thread Benjamin David Lunt
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

2024-02-24 Thread Daniel Henrique Barboza




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

2024-02-24 Thread Stefan Weil
** 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.

2024-02-24 Thread Hao Xiang
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

2024-02-24 Thread Rayhan Faizel
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

2024-02-24 Thread Rayhan Faizel
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)

2024-02-24 Thread Rayhan Faizel
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

2024-02-24 Thread Rayhan Faizel
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

2024-02-24 Thread Rayhan Faizel
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

2024-02-24 Thread Rayhan Faizel
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)

2024-02-24 Thread Rayhan Faizel
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

2024-02-24 Thread Rayhan Faizel
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

2024-02-24 Thread Richard Henderson

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

2024-02-24 Thread Paolo Bonzini
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

2024-02-24 Thread Jason Andryuk
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.

2024-02-24 Thread Hao Xiang
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.

2024-02-24 Thread Hao Xiang
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

2024-02-24 Thread demin.han
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki

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()

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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()

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Inès Varhol
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

2024-02-24 Thread Inès Varhol
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

2024-02-24 Thread Inès Varhol
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

2024-02-24 Thread Inès Varhol
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:]

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
[-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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Peter Maydell
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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:]

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Akihiko Odaki
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

2024-02-24 Thread Het Gala


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

2024-02-24 Thread Akihiko Odaki

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

2024-02-24 Thread Akihiko Odaki

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

2024-02-24 Thread BALATON Zoltan

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

2024-02-24 Thread Minwoo Im
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"

2024-02-24 Thread Bernhard Beschow
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"

2024-02-24 Thread Bernhard Beschow
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

2024-02-24 Thread 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`
* `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

2024-02-24 Thread Bernhard Beschow
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

2024-02-24 Thread Bernhard Beschow
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

2024-02-24 Thread Bernhard Beschow
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

2024-02-24 Thread Bernhard Beschow
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

2024-02-24 Thread Bernhard Beschow



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

2024-02-24 Thread David Parsons
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

2024-02-24 Thread Bernhard Beschow



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

2024-02-24 Thread Akihiko Odaki

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

2024-02-24 Thread demin.han
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

2024-02-24 Thread Hyman Huang
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

2024-02-24 Thread Hyman Huang
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

2024-02-24 Thread Hyman Huang
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

2024-02-24 Thread Hyman Huang
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