Re: [PATCH v3 00/27] target/s390x: pc-relative translation blocks

[Thomas Huth]

On 09/01/2023 21.07, Richard Henderson wrote:

This is the S390 specific changes required to reduce the
amount of translation for address space randomization.

Changes for v3:
   * Rebase and fixup conflicts.

All patches are reviewed.


 Hi Richard,

as far as I can see, this series has not been merged yet? Were there any 
issues left here? ... soft freeze is coming rather sooner than later ... do 
you want me to take this trough my s390x tree, or will you take it through 
your tcg tree?


 Thomas

Re: [RFC PATCH 10/43] target/loongarch: Implement vaddw/vsubw

[gaosong]

Hi, Richard

在 2022/12/25 上午1:48, Richard Henderson 写道:

On 12/24/22 00:16, Song Gao wrote:

+TRANS(vaddwev_h_b, gen_vvv, gen_helper_vaddwev_h_b)
+TRANS(vaddwev_w_h, gen_vvv, gen_helper_vaddwev_w_h)
+TRANS(vaddwev_d_w, gen_vvv, gen_helper_vaddwev_d_w)
+TRANS(vaddwev_q_d, gen_vvv, gen_helper_vaddwev_q_d)
+TRANS(vaddwod_h_b, gen_vvv, gen_helper_vaddwod_h_b)
+TRANS(vaddwod_w_h, gen_vvv, gen_helper_vaddwod_w_h)
+TRANS(vaddwod_d_w, gen_vvv, gen_helper_vaddwod_d_w)
+TRANS(vaddwod_q_d, gen_vvv, gen_helper_vaddwod_q_d)
+TRANS(vsubwev_h_b, gen_vvv, gen_helper_vsubwev_h_b)
+TRANS(vsubwev_w_h, gen_vvv, gen_helper_vsubwev_w_h)
+TRANS(vsubwev_d_w, gen_vvv, gen_helper_vsubwev_d_w)
+TRANS(vsubwev_q_d, gen_vvv, gen_helper_vsubwev_q_d)
+TRANS(vsubwod_h_b, gen_vvv, gen_helper_vsubwod_h_b)
+TRANS(vsubwod_w_h, gen_vvv, gen_helper_vsubwod_w_h)
+TRANS(vsubwod_d_w, gen_vvv, gen_helper_vsubwod_d_w)
+TRANS(vsubwod_q_d, gen_vvv, gen_helper_vsubwod_q_d)


These can be implemented with a combination of vector shift + vector add.


+TRANS(vaddwev_h_bu, gen_vvv, gen_helper_vaddwev_h_bu)
+TRANS(vaddwev_w_hu, gen_vvv, gen_helper_vaddwev_w_hu)
+TRANS(vaddwev_d_wu, gen_vvv, gen_helper_vaddwev_d_wu)
+TRANS(vaddwev_q_du, gen_vvv, gen_helper_vaddwev_q_du)
+TRANS(vaddwod_h_bu, gen_vvv, gen_helper_vaddwod_h_bu)
+TRANS(vaddwod_w_hu, gen_vvv, gen_helper_vaddwod_w_hu)
+TRANS(vaddwod_d_wu, gen_vvv, gen_helper_vaddwod_d_wu)
+TRANS(vaddwod_q_du, gen_vvv, gen_helper_vaddwod_q_du)
+TRANS(vsubwev_h_bu, gen_vvv, gen_helper_vsubwev_h_bu)
+TRANS(vsubwev_w_hu, gen_vvv, gen_helper_vsubwev_w_hu)
+TRANS(vsubwev_d_wu, gen_vvv, gen_helper_vsubwev_d_wu)
+TRANS(vsubwev_q_du, gen_vvv, gen_helper_vsubwev_q_du)
+TRANS(vsubwod_h_bu, gen_vvv, gen_helper_vsubwod_h_bu)
+TRANS(vsubwod_w_hu, gen_vvv, gen_helper_vsubwod_w_hu)
+TRANS(vsubwod_d_wu, gen_vvv, gen_helper_vsubwod_d_wu)
+TRANS(vsubwod_q_du, gen_vvv, gen_helper_vsubwod_q_du)


These can be implemented with a combination of vector and + vector add.


+TRANS(vaddwev_h_bu_b, gen_vvv, gen_helper_vaddwev_h_bu_b)
+TRANS(vaddwev_w_hu_h, gen_vvv, gen_helper_vaddwev_w_hu_h)
+TRANS(vaddwev_d_wu_w, gen_vvv, gen_helper_vaddwev_d_wu_w)
+TRANS(vaddwev_q_du_d, gen_vvv, gen_helper_vaddwev_q_du_d)
+TRANS(vaddwod_h_bu_b, gen_vvv, gen_helper_vaddwod_h_bu_b)
+TRANS(vaddwod_w_hu_h, gen_vvv, gen_helper_vaddwod_w_hu_h)
+TRANS(vaddwod_d_wu_w, gen_vvv, gen_helper_vaddwod_d_wu_w)
+TRANS(vaddwod_q_du_d, gen_vvv, gen_helper_vaddwod_q_du_d)


Likewise.

For an example of how to bundle vector operations, see e.g. 
gen_gvec_rax1 and subroutines in target/arm/translate-a64.c. There are 
many others, but ask if you need more help.



I have some questions:
1 Should we need implement  GVecGen*  for simple gvec instructiosn?
    such as add, sub , or , xor..
2 Should we need implement all fni8/fni4, fniv,  fno?

Thanks
Song Gao

Re: [PATCH] ui: fix crash on serial reset, during init

[Philippe Mathieu-Daudé]

On 20/2/23 08:22, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

For ex, when resetting the xlnx-zcu102 machine:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x50)
* frame #0: 0x10020a740 gd_vc_send_chars(vc=0x0) at
gtk.c:1759:41 [opt]
  frame #1: 0x100636264 qemu_chr_fe_accept_input(be=) at
char-fe.c:159:9 [opt]
  frame #2: 0x1000608e0 cadence_uart_reset_hold [inlined]
uart_rx_reset(s=0x10810a960) at cadence_uart.c:158:5 [opt]
  frame #3: 0x1000608d4 cadence_uart_reset_hold(obj=0x10810a960) at
cadence_uart.c:530:5 [opt]
  frame #4: 0x100580ab4 resettable_phase_hold(obj=0x10810a960,
opaque=0x0, type=) at resettable.c:0 [opt]
  frame #5: 0x10057d1b0 bus_reset_child_foreach(obj=,
cb=(resettable_phase_hold at resettable.c:162), opaque=0x0,
type=RESET_TYPE_COLD) at bus.c:97:13 [opt]
  frame #6: 0x1005809f8 resettable_phase_hold [inlined]
resettable_child_foreach(rc=0x6332d2c0, obj=0x62c1c180,
cb=, opaque=0x0, type=RESET_TYPE_COLD) at
resettable.c:96:9 [opt]
  frame #7: 0x1005809d8 resettable_phase_hold(obj=0x62c1c180,
opaque=0x0, type=RESET_TYPE_COLD) at resettable.c:173:5 [opt]
  frame #8: 0x1005803a0
resettable_assert_reset(obj=0x62c1c180, type=) at
resettable.c:60:5 [opt]
  frame #9: 0x10058027c resettable_reset(obj=0x62c1c180,
type=RESET_TYPE_COLD) at resettable.c:45:5 [opt]

While the chardev is created early, the VirtualConsole is associated
after, during qemu_init_displays().

Signed-off-by: Marc-André Lureau 
Tested-by: Philippe Mathieu-Daudé 
---
  ui/gtk.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

[PATCH] ui: fix crash on serial reset, during init

[marcandre . lureau]

From: Marc-André Lureau 

For ex, when resetting the xlnx-zcu102 machine:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x50)
   * frame #0: 0x10020a740 gd_vc_send_chars(vc=0x0) at
gtk.c:1759:41 [opt]
 frame #1: 0x100636264 qemu_chr_fe_accept_input(be=) at
char-fe.c:159:9 [opt]
 frame #2: 0x1000608e0 cadence_uart_reset_hold [inlined]
uart_rx_reset(s=0x10810a960) at cadence_uart.c:158:5 [opt]
 frame #3: 0x1000608d4 cadence_uart_reset_hold(obj=0x10810a960) at
cadence_uart.c:530:5 [opt]
 frame #4: 0x100580ab4 resettable_phase_hold(obj=0x10810a960,
opaque=0x0, type=) at resettable.c:0 [opt]
 frame #5: 0x10057d1b0 bus_reset_child_foreach(obj=,
cb=(resettable_phase_hold at resettable.c:162), opaque=0x0,
type=RESET_TYPE_COLD) at bus.c:97:13 [opt]
 frame #6: 0x1005809f8 resettable_phase_hold [inlined]
resettable_child_foreach(rc=0x6332d2c0, obj=0x62c1c180,
cb=, opaque=0x0, type=RESET_TYPE_COLD) at
resettable.c:96:9 [opt]
 frame #7: 0x1005809d8 resettable_phase_hold(obj=0x62c1c180,
opaque=0x0, type=RESET_TYPE_COLD) at resettable.c:173:5 [opt]
 frame #8: 0x1005803a0
resettable_assert_reset(obj=0x62c1c180, type=) at
resettable.c:60:5 [opt]
 frame #9: 0x10058027c resettable_reset(obj=0x62c1c180,
type=RESET_TYPE_COLD) at resettable.c:45:5 [opt]

While the chardev is created early, the VirtualConsole is associated
after, during qemu_init_displays().

Signed-off-by: Marc-André Lureau 
Tested-by: Philippe Mathieu-Daudé 
---
 ui/gtk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index fd82e9b1ca..57ae32474c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1783,7 +1783,9 @@ static void gd_vc_chr_accept_input(Chardev *chr)
 VCChardev *vcd = VC_CHARDEV(chr);
 VirtualConsole *vc = vcd->console;
 
-gd_vc_send_chars(vc);
+if (vc) {
+gd_vc_send_chars(vc);
+}
 }
 
 static void gd_vc_chr_set_echo(Chardev *chr, bool echo)
-- 
2.39.1

Re: [PATCH v1 3/4] linux-user: add target to host netlink conversions

[Philippe Mathieu-Daudé]

On 17/2/23 17:35, Mathis Marion wrote:

From: Mathis Marion 

Added conversions for:
- IFLA_MTU
- IFLA_TXQLEN
- IFLA_AF_SPEC AF_INET6 IFLA_INET6_ADDR_GEN_MODE
These relate to the libnl functions rtnl_link_set_mtu,
rtnl_link_set_txqlen, and rtnl_link_inet6_set_addr_gen_mode.

Signed-off-by: Mathis Marion 
---
  linux-user/fd-trans.c | 64 +++
  1 file changed, 64 insertions(+)

diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
index 146faa..aa398098ec 100644
--- a/linux-user/fd-trans.c
+++ b/linux-user/fd-trans.c
@@ -1284,6 +1284,49 @@ static inline abi_long host_to_target_nlmsg_route(struct 
nlmsghdr *nlh,
  return host_to_target_for_each_nlmsg(nlh, len, host_to_target_data_route);
  }
  
+static abi_long target_to_host_for_each_nlattr(struct nlattr *nlattr,

+   size_t len, void *context,


You always pass a NULL context... Do we really need it?


+   abi_long 
(*target_to_host_nlattr)
+(struct nlattr *))
+{
+unsigned short aligned_nla_len;
+abi_long ret;
+
+while (len > sizeof(struct nlattr)) {
+if (tswap16(nlattr->nla_len) < sizeof(struct rtattr) ||
+tswap16(nlattr->nla_len) > len) {
+break;
+}
+nlattr->nla_len = tswap16(nlattr->nla_len);
+nlattr->nla_type = tswap16(nlattr->nla_type);
+ret = target_to_host_nlattr(nlattr);
+if (ret < 0) {


If this fail, guest's nlattr is now inconsistent. Is this OK?


+return ret;
+}
+
+aligned_nla_len = NLA_ALIGN(nlattr->nla_len);
+if (aligned_nla_len >= len) {
+break;
+}
+len -= aligned_nla_len;
+nlattr = (struct nlattr *)(((char *)nlattr) + aligned_nla_len);
+}
+return 0;
+}
+
+static abi_long target_to_host_data_inet6_nlattr(struct nlattr *nlattr)
+{
+switch (nlattr->nla_type) {
+/* uint8_t */
+case QEMU_IFLA_INET6_ADDR_GEN_MODE:
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "Unknown target AF_INET6 type: %d\n",
+  nlattr->nla_type);
+}
+return 0;
+}
+
  static abi_long target_to_host_for_each_rtattr(struct rtattr *rtattr,
 size_t len,
 abi_long 
(*target_to_host_rtattr)
@@ -1314,16 +1357,37 @@ static abi_long target_to_host_for_each_rtattr(struct 
rtattr *rtattr,
  return 0;
  }
  
+static abi_long target_to_host_data_spec_nlattr(struct nlattr *nlattr)

+{
+switch (nlattr->nla_type) {
+case AF_INET6:
+return target_to_host_for_each_nlattr(NLA_DATA(nlattr), 
nlattr->nla_len,
+  NULL,
+  
target_to_host_data_inet6_nlattr);
+default:
+qemu_log_mask(LOG_UNIMP, "Unknown target AF_SPEC type: %d\n",
+  nlattr->nla_type);
+break;
+}
+return 0;
+}
+
  static abi_long target_to_host_data_link_rtattr(struct rtattr *rtattr)
  {
  uint32_t *u32;
  
  switch (rtattr->rta_type) {

  /* uint32_t */
+case QEMU_IFLA_MTU:
+case QEMU_IFLA_TXQLEN:
  case QEMU_IFLA_EXT_MASK:
  u32 = RTA_DATA(rtattr);
  *u32 = tswap32(*u32);
  break;
+case QEMU_IFLA_AF_SPEC:
+return target_to_host_for_each_nlattr(RTA_DATA(rtattr), 
rtattr->rta_len,
+  NULL,
+  target_to_host_data_spec_nlattr);
  default:
  qemu_log_mask(LOG_UNIMP, "Unknown target QEMU_IFLA type: %d\n",
rtattr->rta_type);

Re: [PATCH] xio3130_downstream: Add ACS (Access Control Services) capability

[Philippe Mathieu-Daudé]

Hi Paul,

On 31/1/23 07:30, wlfightup wrote:

When vfio-pci devices are attached to the downstream, pcie acs
capability may be needed, Consistent with physical machine.

It has been tested in our environment, and pcie acs capability
is required in some scenarios.

Claim ACS support in the downstream port to allow
passthrough of individual functions of a device to different
guests (in a nested virt.setting) with VFIO.
Without this patch, all functions of a device, such as all VFs of
an SR/IOV device, will end up in the same IOMMU group.
A similar situation occurs on Windows with Hyper-V.

Signed-off-by: wlfightup 


Please use your real name, "Paul Schlacter "
See https://www.qemu.org/docs/master/devel/submitting-a-patch.html

Cc'ing VFIO maintainers.

Regards,

Phil.


---
  hw/pci-bridge/xio3130_downstream.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index 38a2361fa2..2017cf42a3 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -40,6 +40,8 @@
  #define XIO3130_SSVID_SSID  0
  #define XIO3130_EXP_OFFSET  0x90
  #define XIO3130_AER_OFFSET  0x100
+#define XIO3130_ACS_OFFSET \
+(XIO3130_AER_OFFSET + PCI_ERR_SIZEOF)
  
  static void xio3130_downstream_write_config(PCIDevice *d, uint32_t address,

   uint32_t val, int len)
@@ -111,6 +113,10 @@ static void xio3130_downstream_realize(PCIDevice *d, Error 
**errp)
  goto err;
  }
  
+if (!s->disable_acs) {

+pcie_acs_init(d, XIO3130_ACS_OFFSET);
+}
+
  return;
  
  err:

@@ -137,6 +143,7 @@ static void xio3130_downstream_exitfn(PCIDevice *d)
  static Property xio3130_downstream_props[] = {
  DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
  QEMU_PCIE_SLTCAP_PCP_BITNR, true),
+DEFINE_PROP_BOOL("x-disable-acs", PCIESlot, disable_acs, true),
  DEFINE_PROP_END_OF_LIST()
  };
  

Re: [PATCH v8 0/8] Introduce igb

[Akihiko Odaki]

On 2023/02/20 16:01, Jason Wang wrote:


在 2023/2/6 20:30, Akihiko Odaki 写道:

Hi Jason,

Let me remind that every patches in this series now has Reviewed-by: 
or Acked-by: tag though I forgot to include tags the prior versions of 
this series received to the latest version:



No worries, I can do that.

But when I try, it doesn't apply cleanly on master, are there any 
dependence I missed?


# git am *.eml
Applying: pcie: Introduce pcie_sriov_num_vfs
Applying: e1000: Split header files
error: patch failed: hw/net/e1000_regs.h:470
error: hw/net/e1000_regs.h: patch does not apply
error: patch failed: hw/net/e1000x_common.c:29
error: hw/net/e1000x_common.c: patch does not apply
Patch failed at 0002 e1000: Split header files
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


It is Based-on: <20230201033539.30049-1-akihiko.od...@daynix.com>.
([PATCH v5 00/29] e1000x cleanups (preliminary for IGB))

Please apply the series first.

Regards,
Akihiko Odaki



Thanks




"Introduce igb"
https://lore.kernel.org/qemu-devel/dbbp189mb143365704198dc9a0684dea595...@dbbp189mb1433.eurp189.prod.outlook.com/

"docs/system/devices/igb: Add igb documentation"
https://lore.kernel.org/qemu-devel/741a0975-9f7a-b4bc-9651-cf45f03d1...@kaod.org/

Regards,
Akihiko Odaki

On 2023/02/04 13:36, Akihiko Odaki wrote:

Based-on: <20230201033539.30049-1-akihiko.od...@daynix.com>
([PATCH v5 00/29] e1000x cleanups (preliminary for IGB))

igb is a family of Intel's gigabit ethernet controllers. This series 
implements
82576 emulation in particular. You can see the last patch for the 
documentation.


Note that there is another effort to bring 82576 emulation. This 
series was

developed independently by Sriram Yagnaraman.
https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg04670.html

V7 -> V8:
- Removed obsolete patch
   "hw/net/net_tx_pkt: Introduce net_tx_pkt_get_eth_hdr" (Cédric Le 
Goater)


V6 -> V7:
- Reordered statements in igb_receive_internal() so that checksum 
will be
   calculated only once and it will be more close to 
e1000e_receive_internal().


V5 -> V6:
- Rebased.
- Renamed "test" to "packet" in tests/qtest/e1000e-test.c.
- Fixed Rx logic so that a Rx pool without enough space won't prevent 
other

   pools from receiving, based on Sriram Yagnaraman's work.

V4 -> V5:
- Rebased.
- Squashed patches to copy from e1000e code and modify it.
- Listed the implemented features.
- Added a check for interrupts availablity on PF.
- Fixed the declaration of igb_receive_internal(). (Sriram Yagnaraman)

V3 -> V4:
- Rebased.
- Corrected PCIDevice specified for DMA.

V2 -> V3:
- Rebased.
- Fixed PCIDevice reference in hw/net/igbvf.c.
- Fixed TX packet switching when VM loopback is enabled.
- Fixed VMDq enablement check.
- Fixed RX descriptor length parser.
- Fixed the definitions of RQDPC readers.
- Implemented VLAN VM filter.
- Implemented VT_CTL.Def_PL.
- Implemented the combination of VMDq and RSS.
- Noted that igb is tested with Windows HLK.

V1 -> V2:
- Spun off e1000e general improvements to a distinct series.
- Restored vnet_hdr offload as there seems nothing preventing from that.

Akihiko Odaki (8):
   pcie: Introduce pcie_sriov_num_vfs
   e1000: Split header files
   Intrdocue igb device emulation
   tests/qtest/e1000e-test: Fabricate ethernet header
   tests/qtest/libqos/e1000e: Export macreg functions
   igb: Introduce qtest for igb device
   tests/avocado: Add igb test
   docs/system/devices/igb: Add igb documentation

  MAINTAINERS   |    9 +
  docs/system/device-emulation.rst  |    1 +
  docs/system/devices/igb.rst   |   71 +
  hw/net/Kconfig    |    5 +
  hw/net/e1000.c    |    1 +
  hw/net/e1000_common.h |  102 +
  hw/net/e1000_regs.h   |  927 +---
  hw/net/e1000e.c   |    3 +-
  hw/net/e1000e_core.c  |    1 +
  hw/net/e1000x_common.c    |    1 +
  hw/net/e1000x_common.h    |   74 -
  hw/net/e1000x_regs.h  |  940 
  hw/net/igb.c  |  612 +++
  hw/net/igb_common.h   |  146 +
  hw/net/igb_core.c | 4043 +
  hw/net/igb_core.h |  144 +
  hw/net/igb_regs.h |  648 +++
  hw/net/igbvf.c    |  327 ++
  hw/net/meson.build    |    2 +
  hw/net/trace-events   |   32 +
  hw/pci/pcie_sriov.c   |    5 +
  include/hw/pci/pcie_sriov.h   |    3 +
  

Re: [PATCH v8 0/8] Introduce igb

[Jason Wang]

在 2023/2/6 20:30, Akihiko Odaki 写道:

Hi Jason,

Let me remind that every patches in this series now has Reviewed-by: 
or Acked-by: tag though I forgot to include tags the prior versions of 
this series received to the latest version:



No worries, I can do that.

But when I try, it doesn't apply cleanly on master, are there any 
dependence I missed?


# git am *.eml
Applying: pcie: Introduce pcie_sriov_num_vfs
Applying: e1000: Split header files
error: patch failed: hw/net/e1000_regs.h:470
error: hw/net/e1000_regs.h: patch does not apply
error: patch failed: hw/net/e1000x_common.c:29
error: hw/net/e1000x_common.c: patch does not apply
Patch failed at 0002 e1000: Split header files
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Thanks




"Introduce igb"
https://lore.kernel.org/qemu-devel/dbbp189mb143365704198dc9a0684dea595...@dbbp189mb1433.eurp189.prod.outlook.com/ 



"docs/system/devices/igb: Add igb documentation"
https://lore.kernel.org/qemu-devel/741a0975-9f7a-b4bc-9651-cf45f03d1...@kaod.org/ 



Regards,
Akihiko Odaki

On 2023/02/04 13:36, Akihiko Odaki wrote:

Based-on: <20230201033539.30049-1-akihiko.od...@daynix.com>
([PATCH v5 00/29] e1000x cleanups (preliminary for IGB))

igb is a family of Intel's gigabit ethernet controllers. This series 
implements
82576 emulation in particular. You can see the last patch for the 
documentation.


Note that there is another effort to bring 82576 emulation. This 
series was

developed independently by Sriram Yagnaraman.
https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg04670.html

V7 -> V8:
- Removed obsolete patch
   "hw/net/net_tx_pkt: Introduce net_tx_pkt_get_eth_hdr" (Cédric Le 
Goater)


V6 -> V7:
- Reordered statements in igb_receive_internal() so that checksum 
will be
   calculated only once and it will be more close to 
e1000e_receive_internal().


V5 -> V6:
- Rebased.
- Renamed "test" to "packet" in tests/qtest/e1000e-test.c.
- Fixed Rx logic so that a Rx pool without enough space won't prevent 
other

   pools from receiving, based on Sriram Yagnaraman's work.

V4 -> V5:
- Rebased.
- Squashed patches to copy from e1000e code and modify it.
- Listed the implemented features.
- Added a check for interrupts availablity on PF.
- Fixed the declaration of igb_receive_internal(). (Sriram Yagnaraman)

V3 -> V4:
- Rebased.
- Corrected PCIDevice specified for DMA.

V2 -> V3:
- Rebased.
- Fixed PCIDevice reference in hw/net/igbvf.c.
- Fixed TX packet switching when VM loopback is enabled.
- Fixed VMDq enablement check.
- Fixed RX descriptor length parser.
- Fixed the definitions of RQDPC readers.
- Implemented VLAN VM filter.
- Implemented VT_CTL.Def_PL.
- Implemented the combination of VMDq and RSS.
- Noted that igb is tested with Windows HLK.

V1 -> V2:
- Spun off e1000e general improvements to a distinct series.
- Restored vnet_hdr offload as there seems nothing preventing from that.

Akihiko Odaki (8):
   pcie: Introduce pcie_sriov_num_vfs
   e1000: Split header files
   Intrdocue igb device emulation
   tests/qtest/e1000e-test: Fabricate ethernet header
   tests/qtest/libqos/e1000e: Export macreg functions
   igb: Introduce qtest for igb device
   tests/avocado: Add igb test
   docs/system/devices/igb: Add igb documentation

  MAINTAINERS   |    9 +
  docs/system/device-emulation.rst  |    1 +
  docs/system/devices/igb.rst   |   71 +
  hw/net/Kconfig    |    5 +
  hw/net/e1000.c    |    1 +
  hw/net/e1000_common.h |  102 +
  hw/net/e1000_regs.h   |  927 +---
  hw/net/e1000e.c   |    3 +-
  hw/net/e1000e_core.c  |    1 +
  hw/net/e1000x_common.c    |    1 +
  hw/net/e1000x_common.h    |   74 -
  hw/net/e1000x_regs.h  |  940 
  hw/net/igb.c  |  612 +++
  hw/net/igb_common.h   |  146 +
  hw/net/igb_core.c | 4043 +
  hw/net/igb_core.h |  144 +
  hw/net/igb_regs.h |  648 +++
  hw/net/igbvf.c    |  327 ++
  hw/net/meson.build    |    2 +
  hw/net/trace-events   |   32 +
  hw/pci/pcie_sriov.c   |    5 +
  include/hw/pci/pcie_sriov.h   |    3 +
  .../org.centos/stream/8/x86_64/test-avocado   |    1 +
  tests/avocado/igb.py  |   38 +
  tests/qtest/e1000e-test.c |   25 +-
  tests/qtest/fuzz/generic_fuzz_configs.h   |    5 +
  tests/qtest/igb-test.c   

[PATCH] Updated the FSF address to

[Khadija Kamran]

From: Khadija Kamran 

The Free Software Foundation moved to a new address and some sources in QEMU 
referred to their old location.
The address should be updated and replaced to a pointer to 

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/379

Signed-off-by: Khadija Kamran 
---
 contrib/gitdm/filetypes.txt | 3 +--
 hw/scsi/viosrp.h| 3 +--
 hw/sh4/sh7750_regs.h| 3 +--
 include/hw/arm/raspi_platform.h | 3 +--
 include/qemu/uri.h  | 3 +--
 tests/qemu-iotests/022  | 4 +---
 tests/unit/rcutorture.c | 3 +--
 tests/unit/test-rcu-list.c  | 3 +--
 util/uri.c  | 3 +--
 9 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/contrib/gitdm/filetypes.txt b/contrib/gitdm/filetypes.txt
index d2d6f6db8d..b1d01c0992 100644
--- a/contrib/gitdm/filetypes.txt
+++ b/contrib/gitdm/filetypes.txt
@@ -12,8 +12,7 @@
 # GNU Library General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+# along with this program. If not, see .
 #
 # Authors : Gregorio Robles 
 # Authors : Germán Póo-Caamaño 
diff --git a/hw/scsi/viosrp.h b/hw/scsi/viosrp.h
index e5f9768e8f..58c29aa925 100644
--- a/hw/scsi/viosrp.h
+++ b/hw/scsi/viosrp.h
@@ -16,8 +16,7 @@
 /* GNU General Public License for more details.  */
 /*   */
 /* You should have received a copy of the GNU General Public License */
-/* along with this program; if not, write to the Free Software   */
-/* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
+/* along with this program. If not, see . */
 /*   */
 /*   */
 /* This file contains structures and definitions for IBM RPA (RS/6000*/
diff --git a/hw/sh4/sh7750_regs.h b/hw/sh4/sh7750_regs.h
index beb571d5e9..94043431e6 100644
--- a/hw/sh4/sh7750_regs.h
+++ b/hw/sh4/sh7750_regs.h
@@ -22,8 +22,7 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
  * General Public License for more details. You should have received
  * a copy of the GNU General Public License along with RTEMS; see
- * file COPYING. If not, write to the Free Software Foundation, 675
- * Mass Ave, Cambridge, MA 02139, USA.
+ * file COPYING. If not, see .
  *
  * As a special exception, including RTEMS header files in a file,
  * instantiating RTEMS generics or templates, or linking other files
diff --git a/include/hw/arm/raspi_platform.h b/include/hw/arm/raspi_platform.h
index e0e6c8ce94..4a56dd4b89 100644
--- a/include/hw/arm/raspi_platform.h
+++ b/include/hw/arm/raspi_platform.h
@@ -18,8 +18,7 @@
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ * along with this program. If not, see .
  *
  * Various undocumented addresses and names come from Herman Hermitage's VC4
  * documentation:
diff --git a/include/qemu/uri.h b/include/qemu/uri.h
index d201c61260..cf8ec70356 100644
--- a/include/qemu/uri.h
+++ b/include/qemu/uri.h
@@ -41,8 +41,7 @@
  * Lesser General Public License for more details.
  *
  * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ * License along with this library. If not, see 
.
  *
  * Authors:
  *Richard W.M. Jones 
diff --git a/tests/qemu-iotests/022 b/tests/qemu-iotests/022
index a116cfe255..d98d1ea90f 100755
--- a/tests/qemu-iotests/022
+++ b/tests/qemu-iotests/022
@@ -16,9 +16,7 @@
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
-# USA
+# along with this program. If not, see .
 #
 
 # creator
diff --git a/tests/unit/rcutorture.c b/tests/unit/rcutorture.c
index 495a4e6f42..7662081683 100644
--- a/tests/unit/rcutorture.c
+++ b/tests/unit/rcutorture.c
@@ -50,8 +50,7 @@
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free 

Re: [PATCH RESEND 04/18] i386/cpu: Fix number of addressable IDs in CPUID.04H

[Xiaoyao Li]

On 2/13/2023 5:36 PM, Zhao Liu wrote:

For i-cache and d-cache, the maximum IDs for CPUs sharing cache (
CPUID.04H.00H:EAX[bits 25:14] and CPUID.04H.01H:EAX[bits 25:14]) are
both 0, and this means i-cache and d-cache are shared in the SMT level.
This is correct if there's single thread per core, but is wrong for the
hyper threading case (one core contains multiple threads) since the
i-cache and d-cache are shared in the core level other than SMT level.

Therefore, in order to be compatible with both multi-threaded and
single-threaded situations, we should set i-cache and d-cache be shared
at the core level by default.


It's true for VM only when the exactly HW topology is configured to VM. 
i.e., two virtual LPs of one virtual CORE are pinned to two physical LPs 
that of one physical CORE. Otherwise it's incorrect for VM.


for example. given a VM of 4 threads and 2 cores. If not pinning the 4 
threads to physical 4 LPs of 2 CORES. It's likely each vcpu running on a 
LP of different physical cores. So no vcpu shares L1i/L1d cache at core 
level.

Re: [PATCH 3/2] hw/timer: Rename ptimer_state -> PTimer

[Thomas Huth]

On 17/02/2023 22.58, Philippe Mathieu-Daudé wrote:

Remove a pointless cast in ptimer_tick() and rename 'ptimer_state'
as 'PTimer' to follow the Structure naming convention.

See docs/devel/style.rst:

   Variables are lower_case_with_underscores; easy to type and
   read.  Structured type names are in CamelCase; harder to type
   but standing out.  Enum type names and function type names
   should also be in CamelCase.  Scalar type names are
   lower_case_with_underscores_ending_with_a_t, like the POSIX
   uint64_t and family.

Mechanical change doing:

   $ sed -i -e s/ptimer_state/PTimer/g \
   $(git grep -l ptimer_state)

Suggested-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---

...

@@ -154,7 +154,7 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
  
  static void ptimer_tick(void *opaque)

  {
-ptimer_state *s = (ptimer_state *)opaque;
+PTimer *s = opaque;


I like that you also removed the useless cast here.


  bool trigger = true;
  
  /*

@@ -198,7 +198,7 @@ static void ptimer_tick(void *opaque)
  ptimer_transaction_commit(s);
  }
  
-uint64_t ptimer_get_count(ptimer_state *s)

+uint64_t ptimer_get_count(PTimer *s)
  {
  uint64_t counter;
  
@@ -294,7 +294,7 @@ uint64_t ptimer_get_count(ptimer_state *s)

  return counter;
  }
  
-void ptimer_set_count(ptimer_state *s, uint64_t count)

+void ptimer_set_count(PTimer *s, uint64_t count)
  {
  assert(s->in_transaction);
  s->delta = count;
@@ -303,7 +303,7 @@ void ptimer_set_count(ptimer_state *s, uint64_t count)
  }
  }
  
-void ptimer_run(ptimer_state *s, int oneshot)

+void ptimer_run(PTimer *s, int oneshot)
  {
  bool was_disabled = !s->enabled;
  
@@ -323,7 +323,7 @@ void ptimer_run(ptimer_state *s, int oneshot)
  
  /* Pause a timer.  Note that this may cause it to "lose" time, even if it

 is immediately restarted.  */
-void ptimer_stop(ptimer_state *s)
+void ptimer_stop(PTimer *s)
  {
  assert(s->in_transaction);
  
@@ -337,7 +337,7 @@ void ptimer_stop(ptimer_state *s)

  }
  
  /* Set counter increment interval in nanoseconds.  */

-void ptimer_set_period(ptimer_state *s, int64_t period)
+void ptimer_set_period(PTimer *s, int64_t period)
  {
  assert(s->in_transaction);
  s->delta = ptimer_get_count(s);
@@ -349,7 +349,7 @@ void ptimer_set_period(ptimer_state *s, int64_t period)
  }
  
  /* Set counter increment interval from a Clock */

-void ptimer_set_period_from_clock(ptimer_state *s, const Clock *clk,
+void ptimer_set_period_from_clock(PTimer *s, const Clock *clk,
unsigned int divisor)
  {
  /*
@@ -382,7 +382,7 @@ void ptimer_set_period_from_clock(ptimer_state *s, const 
Clock *clk,
  }
  
  /* Set counter frequency in Hz.  */

-void ptimer_set_freq(ptimer_state *s, uint32_t freq)
+void ptimer_set_freq(PTimer *s, uint32_t freq)
  {
  assert(s->in_transaction);
  s->delta = ptimer_get_count(s);
@@ -395,7 +395,7 @@ void ptimer_set_freq(ptimer_state *s, uint32_t freq)
  
  /* Set the initial countdown value.  If reload is nonzero then also set

 count = limit.  */
-void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
+void ptimer_set_limit(PTimer *s, uint64_t limit, int reload)
  {
  assert(s->in_transaction);
  s->limit = limit;
@@ -406,19 +406,19 @@ void ptimer_set_limit(ptimer_state *s, uint64_t limit, 
int reload)
  }
  }
  
-uint64_t ptimer_get_limit(ptimer_state *s)

+uint64_t ptimer_get_limit(PTimer *s)
  {
  return s->limit;
  }
  
-void ptimer_transaction_begin(ptimer_state *s)

+void ptimer_transaction_begin(PTimer *s)
  {
  assert(!s->in_transaction);
  s->in_transaction = true;
  s->need_reload = false;
  }
  
-void ptimer_transaction_commit(ptimer_state *s)

+void ptimer_transaction_commit(PTimer *s)
  {
  assert(s->in_transaction);
  /*
@@ -442,27 +442,27 @@ const VMStateDescription vmstate_ptimer = {
  .version_id = 1,
  .minimum_version_id = 1,
  .fields = (VMStateField[]) {
-VMSTATE_UINT8(enabled, ptimer_state),
-VMSTATE_UINT64(limit, ptimer_state),
-VMSTATE_UINT64(delta, ptimer_state),
-VMSTATE_UINT32(period_frac, ptimer_state),
-VMSTATE_INT64(period, ptimer_state),
-VMSTATE_INT64(last_event, ptimer_state),
-VMSTATE_INT64(next_event, ptimer_state),
-VMSTATE_TIMER_PTR(timer, ptimer_state),
+VMSTATE_UINT8(enabled, PTimer),
+VMSTATE_UINT64(limit, PTimer),
+VMSTATE_UINT64(delta, PTimer),
+VMSTATE_UINT32(period_frac, PTimer),
+VMSTATE_INT64(period, PTimer),
+VMSTATE_INT64(last_event, PTimer),
+VMSTATE_INT64(next_event, PTimer),
+VMSTATE_TIMER_PTR(timer, PTimer),
  VMSTATE_END_OF_LIST()
  }
  };
  
-ptimer_state *ptimer_init(ptimer_cb callback, void *callback_opaque,

+PTimer *ptimer_init(ptimer_cb callback, void *callback_opaque,
uint8_t policy_mask)


Just a 

Re: [PATCH v2 0/7] Python: Drop support for Python 3.6

[Thomas Huth]

On 17/02/2023 21.46, John Snow wrote:

On Thu, Feb 16, 2023 at 5:58 AM Thomas Huth  wrote:


On 15/02/2023 20.05, Markus Armbruster wrote:

The discussion under PATCH 6 makes me think there's a bit of confusion
about the actual impact of dropping support for Python 3.6.  Possibly
because it's spelled out in the commit message of PATCH 7.  Let me
summarize it in one sentence:

  *** All supported host systems continue to work ***

Evidence: CI remains green.


The CI remains green since one of the patches disabled the building of the
docs on CentOS 8. That's not how I'd describe "continue to work", at least
not in the same extend as before.


On some supported host systems, different packages need to be installed.
On CentOS 8, for instance, we need to install Python 3.8.13 or 3.9.16
instead of 3.6.8.  Let me stress again: same repository, different
package.  No downsides I can see.

The *one* exception is Sphinx on CentOS 8.  CentOS 8 does not ship a
version of Sphinx that works with Python 3.7 or newer.  This series
proposes to simply stop building the docs there, unless the user
provides a suitable version of Sphinx (which is easy enough with pip).


I think we've all understood that. The thing that you obviously did not
understood: This breaks our support statement.
I'm pretty sure that you could also build the whole QEMU suite successfully
on an ancient CentOS 7 or Ubuntu 18.04 system if you manually install a
newer version of GCC and some of the required libraries first. But that's
not how we understand our support statement.

Sure, you can argue that you can use "pip install" to get a newer version of
Sphinx on RHEL 8 / CentOS 8 to continue building the docs there - but is
that really that much different from installing a newer version of GCC and
libraries on an ancient distro that we do not officially support anymore?
I'd say no. You also have to consider that not every build host has access
to the internet, maybe some companies only have an internal mirror of the
distro packages in their intranet (I remember some discussion about such a
case in the past) - so while you were perfectly fine to build the whole of
QEMU on a CentOS 8 there before this change, you could now not build parts
of QEMU anymore there due to the missing possibility to run "pip install"
without full internet connection.


There are good points elsewhere in this thread and I am taking notes,
but this critique caught my eye as something I was not specifically
planning around, so I wanted to get an elaboration here if I may.

Do we have a support statement for this? I find this critique somewhat
surprising -- If we don't have internet, how did we get the other 20
to 30 dependencies needed to build QEMU? To what extent are we
*required* to preserve a build that works without internet access?


It's not written in stone, but I saw it this way: If I have a complete 
mirror of a distro repository in my intrAnet, I can use that mirror to set 
up a QEMU build host system that has no access to the internet. Or maybe 
think of a DVD image(s) with all distro packages that you use to install a 
host without network access (and you copy the QEMU tarball there via USB 
stick). I think it's not that uncommon to have such scenarios out there.


For example, do you remember that SDL 1.2 discussion a some years ago? See:

 https://www.mail-archive.com/qemu-devel@nongnu.org/msg631628.html

It was not exactly the same situation, since those folks were even unable to 
install a SDL2-devel package on their pre-installed hosts, though it was 
theoretically available as an update in their distro, but I think it gives 
an impression of what people are using and expecting out there... (and no, 
I'm not happy with this, I'd also rather love if we could move faster in the 
QEMU project sometimes).


 Thomas

Re: [PATCH RESEND 18/18] i386: Add new property to control L2 cache topo in CPUID.04H

[wangyanan (Y)]

在 2023/2/20 10:49, Zhao Liu 写道:

On Fri, Feb 17, 2023 at 05:08:31PM +0800, wangyanan (Y) wrote:

Date: Fri, 17 Feb 2023 17:08:31 +0800
From: "wangyanan (Y)" 
Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
  cache topo in CPUID.04H

在 2023/2/17 15:26, Zhao Liu 写道:

On Fri, Feb 17, 2023 at 12:07:01PM +0800, wangyanan (Y) wrote:

Date: Fri, 17 Feb 2023 12:07:01 +0800
From: "wangyanan (Y)" 
Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
   cache topo in CPUID.04H

在 2023/2/17 11:35, Zhao Liu 写道:

On Thu, Feb 16, 2023 at 09:14:54PM +0800, wangyanan (Y) wrote:

Date: Thu, 16 Feb 2023 21:14:54 +0800
From: "wangyanan (Y)" 
Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
cache topo in CPUID.04H

在 2023/2/13 17:36, Zhao Liu 写道:

From: Zhao Liu 

The property x-l2-cache-topo will be used to change the L2 cache
topology in CPUID.04H.

Now it allows user to set the L2 cache is shared in core level or
cluster level.

If user passes "-cpu x-l2-cache-topo=[core|cluster]" then older L2 cache
topology will be overrided by the new topology setting.

Currently x-l2-cache-topo only defines the share level *globally*.

Yes, will set for all CPUs.


I'm thinking how we can make the property more powerful so that it
can specify which CPUs share l2 on core level and which CPUs share
l2 on cluster level.

What would Intel's Hybrid CPUs do? Determine the l2 share level
is core or cluster according to the CPU core type (Atom or Core)?
While ARM does not have the core type concept but have CPUs
that l2 is shared on different levels in the same system.

For example, Alderlake's "core" shares 1 L2 per core and every 4 "atom"s
share 1 L2. For this case, we can set the topology as:

cluster0 has 1 "core" and cluster1 has 4 "atom". Then set L2 shared on
cluster level.

Since cluster0 has only 1 "core" type core, then L2 per "core" works.

Not sure if this idea can be applied to arm?

For a CPU topopoly where we have 2 clusters totally, 2 cores in cluster0
have their own L1/L2 cache and 2 threads in each core, 4 cores in cluster1
share one L2 cache and 1 thread in each core. The global way does not
work well.

What about defining something general, which looks like -numa config:
-cache-topo cache=l2, share_level="core", cpus='0-3'
-cache-topo cache=l2, share_level="cluster", cpus='4-7'

Hi Yanan, here it may be necessary to check whether the cpu index set
in "cpus" is reasonable through the specific cpu topology.

Yes, the validity of the cache topo configs from the users should be
check in machine_parse_cache_topo ( if we will have this func).
It's not a big deal, we always need the validity checks.

I guess that verification needs to build up the full cpu topology, as
done in another hybrid RFC. So, should the cpu-topology.h related
patches in that RFC be split out and sent first?

Which patches, do you mean the re-work/generalization of smp?
Will it be reasonable if they are sent separately without the hybrid
introduction? I'm not yet sure if -cache must need the cpu-topology.h
related things, maybe current upstream QEMU code is enough.

In summary:
1、There can not be the same cpus in different "cpus" list.
2、A combination of all the "cpus" list should *just* cover all the CPUs
in the machine
3、Most importantly, cpus in the same cluster must be set with the
same cache "share_level" (core or cluster) and cpus in the same core
must also be set with the same cache "share_level".

Got it, thx.


For example, core0 has 2 CPUs: cpu0 and cpu1, and core1 has 2 CPUs: cpu2
and cpu3, then set l2 as:

-cache-topo cache=l2, share_level="core", cpus='0-2'
-cache-topo cache=l2, share_level="core", cpus='3'

Whether this command is legal depends on the meaning we give to the
parameter "cpu":

s/cpus/cpu.
It means all the afftected CPUs, e.g, the second case.

1. If "cpu" means all cpus share the cache set in this command, then
this command should fail since cpu2 and cpu3 are in a core.

2. If "cpu" means the affected cpus, then this command should find the
cores they belong to according to the cpu topology, and set L2 for those
cores. This command may return success.

What about removing share_level and ask "cpu" to mean all the sharing
cpus to avoid checking the cpu topology?

Then the above example should be:

-cache-topo cache=l2, cpus='0-1'
-cache-topo cache=l2, cpus='2-3'

Sorry, I dont understand how we will know the cache share_level of
cpus '0-1' or '2-3'. What will the CLIs will be like if we change the
belows CLIs by removing the "share_level" params.

-cache-topo cache=l2, share_level="core", cpus='0-3'
-cache-topo cache=l2, share_level="cluster", cpus='4-7'

This decouples cpu topology and cache topology completely and very
simple. In this way, determining the cache by specifying the shared cpu
is similar to that in x86 CPUID.04H.

But the price of simplicity is we may build a cache topology that doesn't
match the reality.

But if the cache topology must be set 

[PULL 6/7] tests/tcg/linux-test: Add linux-fork-trap test

[Richard Henderson]

From: Ilya Leoshkevich 

Check that dying due to a signal does not deadlock.

Reviewed-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Signed-off-by: Ilya Leoshkevich 
Message-Id: <20230214140829.45392-5-...@linux.ibm.com>
Signed-off-by: Richard Henderson 
---
 tests/tcg/multiarch/linux/linux-fork-trap.c | 51 +
 1 file changed, 51 insertions(+)
 create mode 100644 tests/tcg/multiarch/linux/linux-fork-trap.c

diff --git a/tests/tcg/multiarch/linux/linux-fork-trap.c 
b/tests/tcg/multiarch/linux/linux-fork-trap.c
new file mode 100644
index 00..2bfef800c3
--- /dev/null
+++ b/tests/tcg/multiarch/linux/linux-fork-trap.c
@@ -0,0 +1,51 @@
+/*
+ * Test that a fork()ed process terminates after __builtin_trap().
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int main(void)
+{
+struct rlimit nodump;
+pid_t err, pid;
+int wstatus;
+
+pid = fork();
+assert(pid != -1);
+if (pid == 0) {
+/* We are about to crash on purpose; disable core dumps. */
+if (getrlimit(RLIMIT_CORE, )) {
+return EXIT_FAILURE;
+}
+nodump.rlim_cur = 0;
+if (setrlimit(RLIMIT_CORE, )) {
+return EXIT_FAILURE;
+}
+/*
+ * An alternative would be to dereference a NULL pointer, but that
+ * would be an UB in C.
+ */
+printf("about to trigger fault...\n");
+#if defined(__MICROBLAZE__)
+/*
+ * gcc emits "bri 0", which is an endless loop.
+ * Take glibc's ABORT_INSTRUCTION.
+ */
+asm volatile("brki r0,-1");
+#else
+__builtin_trap();
+#endif
+}
+err = waitpid(pid, , 0);
+assert(err == pid);
+assert(WIFSIGNALED(wstatus));
+printf("faulting thread exited cleanly\n");
+
+return EXIT_SUCCESS;
+}
-- 
2.34.1

[PULL 1/7] accel/tcg: Allow the second page of an instruction to be MMIO

[Richard Henderson]

If an instruction straddles a page boundary, and the first page
was ram, but the second page was MMIO, we would abort.  Handle
this as if both pages are MMIO, by setting the ram_addr_t for
the first page to -1.

Reported-by: Sid Manning 
Reported-by: Jørgen Hansen 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 accel/tcg/translator.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index ef5193c67e..1cf404ced0 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -176,8 +176,16 @@ static void *translator_access(CPUArchState *env, 
DisasContextBase *db,
 if (host == NULL) {
 tb_page_addr_t phys_page =
 get_page_addr_code_hostp(env, base, >host_addr[1]);
-/* We cannot handle MMIO as second page. */
-assert(phys_page != -1);
+
+/*
+ * If the second page is MMIO, treat as if the first page
+ * was MMIO as well, so that we do not cache the TB.
+ */
+if (unlikely(phys_page == -1)) {
+tb_set_page_addr0(tb, -1);
+return NULL;
+}
+
 tb_set_page_addr1(tb, phys_page);
 #ifdef CONFIG_USER_ONLY
 page_protect(end);
-- 
2.34.1

[PULL 3/7] linux-user: Always exit from exclusive state in fork_end()

[Richard Henderson]

From: Ilya Leoshkevich 

fork()ed processes currently start with
current_cpu->in_exclusive_context set, which is, strictly speaking, not
correct, but does not cause problems (even assertion failures).

With one of the next patches, the code begins to rely on this value, so
fix it by always calling end_exclusive() in fork_end().

Reviewed-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Signed-off-by: Ilya Leoshkevich 
Message-Id: <20230214140829.45392-2-...@linux.ibm.com>
Signed-off-by: Richard Henderson 
---
 linux-user/main.c| 10 ++
 linux-user/syscall.c |  1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 4290651c3c..4ff30ff980 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -161,13 +161,15 @@ void fork_end(int child)
 }
 qemu_init_cpu_list();
 gdbserver_fork(thread_cpu);
-/* qemu_init_cpu_list() takes care of reinitializing the
- * exclusive state, so we don't need to end_exclusive() here.
- */
 } else {
 cpu_list_unlock();
-end_exclusive();
 }
+/*
+ * qemu_init_cpu_list() reinitialized the child exclusive state, but we
+ * also need to keep current_cpu consistent, so call end_exclusive() for
+ * both child and parent.
+ */
+end_exclusive();
 }
 
 __thread CPUState *thread_cpu;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1e868e9b0e..a6c426d73c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6752,6 +6752,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, 
abi_ulong newsp,
 cpu_clone_regs_parent(env, flags);
 fork_end(0);
 }
+g_assert(!cpu_in_exclusive_context(cpu));
 }
 return ret;
 }
-- 
2.34.1

[PULL 0/7] tcg patch queue

[Richard Henderson]

The linux-user patches are on the tcg-ish side of user-only
emulation, rather than the syscall-ish side, so queuing here.
Solving the deadlock issue is quite important vs timeouts.


r~


The following changes since commit 6dffbe36af79e26a4d23f94a9a1c1201de99c261:

  Merge tag 'migration-20230215-pull-request' of 
https://gitlab.com/juan.quintela/qemu into staging (2023-02-16 13:09:51 +)

are available in the Git repository at:

  https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20230219

for you to fetch changes up to 2f5b4792c0220920831ac84f94c3435b14791857:

  target/microblaze: Add gdbstub xml (2023-02-19 16:12:26 -1000)


tcg: Allow first half of insn in ram, and second half in mmio
linux-user/sparc: SIGILL for unknown trap vectors
linux-user/microblaze: SIGILL for privileged insns
linux-user: Fix deadlock while exiting due to signal
target/microblaze: Add gdbstub xml


Ilya Leoshkevich (4):
  linux-user: Always exit from exclusive state in fork_end()
  cpus: Make {start,end}_exclusive() recursive
  linux-user/microblaze: Handle privileged exception
  tests/tcg/linux-test: Add linux-fork-trap test

Richard Henderson (3):
  accel/tcg: Allow the second page of an instruction to be MMIO
  linux-user/sparc: Raise SIGILL for all unhandled software traps
  target/microblaze: Add gdbstub xml

 configs/targets/microblaze-linux-user.mak   |  1 +
 configs/targets/microblaze-softmmu.mak  |  1 +
 configs/targets/microblazeel-linux-user.mak |  1 +
 configs/targets/microblazeel-softmmu.mak|  1 +
 include/hw/core/cpu.h   |  4 +-
 target/microblaze/cpu.h |  2 +
 accel/tcg/translator.c  | 12 +-
 cpus-common.c   | 12 +-
 linux-user/main.c   | 10 +++--
 linux-user/microblaze/cpu_loop.c| 10 -
 linux-user/sparc/cpu_loop.c |  8 
 linux-user/syscall.c|  1 +
 target/microblaze/cpu.c |  7 ++-
 target/microblaze/gdbstub.c | 51 --
 tests/tcg/multiarch/linux/linux-fork-trap.c | 51 ++
 gdb-xml/microblaze-core.xml | 67 +
 gdb-xml/microblaze-stack-protect.xml| 12 ++
 17 files changed, 224 insertions(+), 27 deletions(-)
 create mode 100644 tests/tcg/multiarch/linux/linux-fork-trap.c
 create mode 100644 gdb-xml/microblaze-core.xml
 create mode 100644 gdb-xml/microblaze-stack-protect.xml

[PULL 7/7] target/microblaze: Add gdbstub xml

[Richard Henderson]

Mirroring the upstream gdb xml files, the two stack boundary
registers are separated out.

Reviewed-by: Edgar E. Iglesias 
Signed-off-by: Richard Henderson 
---
 configs/targets/microblaze-linux-user.mak   |  1 +
 configs/targets/microblaze-softmmu.mak  |  1 +
 configs/targets/microblazeel-linux-user.mak |  1 +
 configs/targets/microblazeel-softmmu.mak|  1 +
 target/microblaze/cpu.h |  2 +
 target/microblaze/cpu.c |  7 ++-
 target/microblaze/gdbstub.c | 51 +++-
 gdb-xml/microblaze-core.xml | 67 +
 gdb-xml/microblaze-stack-protect.xml| 12 
 9 files changed, 128 insertions(+), 15 deletions(-)
 create mode 100644 gdb-xml/microblaze-core.xml
 create mode 100644 gdb-xml/microblaze-stack-protect.xml

diff --git a/configs/targets/microblaze-linux-user.mak 
b/configs/targets/microblaze-linux-user.mak
index 4249a37f65..0a2322c249 100644
--- a/configs/targets/microblaze-linux-user.mak
+++ b/configs/targets/microblaze-linux-user.mak
@@ -3,3 +3,4 @@ TARGET_SYSTBL_ABI=common
 TARGET_SYSTBL=syscall.tbl
 TARGET_BIG_ENDIAN=y
 TARGET_HAS_BFLT=y
+TARGET_XML_FILES=gdb-xml/microblaze-core.xml 
gdb-xml/microblaze-stack-protect.xml
diff --git a/configs/targets/microblaze-softmmu.mak 
b/configs/targets/microblaze-softmmu.mak
index 8385e2d333..e84c0cc728 100644
--- a/configs/targets/microblaze-softmmu.mak
+++ b/configs/targets/microblaze-softmmu.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=microblaze
 TARGET_BIG_ENDIAN=y
 TARGET_SUPPORTS_MTTCG=y
 TARGET_NEED_FDT=y
+TARGET_XML_FILES=gdb-xml/microblaze-core.xml 
gdb-xml/microblaze-stack-protect.xml
diff --git a/configs/targets/microblazeel-linux-user.mak 
b/configs/targets/microblazeel-linux-user.mak
index d0e775d840..270743156a 100644
--- a/configs/targets/microblazeel-linux-user.mak
+++ b/configs/targets/microblazeel-linux-user.mak
@@ -2,3 +2,4 @@ TARGET_ARCH=microblaze
 TARGET_SYSTBL_ABI=common
 TARGET_SYSTBL=syscall.tbl
 TARGET_HAS_BFLT=y
+TARGET_XML_FILES=gdb-xml/microblaze-core.xml 
gdb-xml/microblaze-stack-protect.xml
diff --git a/configs/targets/microblazeel-softmmu.mak 
b/configs/targets/microblazeel-softmmu.mak
index af40391f2f..9b688036bd 100644
--- a/configs/targets/microblazeel-softmmu.mak
+++ b/configs/targets/microblazeel-softmmu.mak
@@ -1,3 +1,4 @@
 TARGET_ARCH=microblaze
 TARGET_SUPPORTS_MTTCG=y
 TARGET_NEED_FDT=y
+TARGET_XML_FILES=gdb-xml/microblaze-core.xml 
gdb-xml/microblaze-stack-protect.xml
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 1e84dd8f47..e541fbb0b3 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -367,6 +367,8 @@ hwaddr mb_cpu_get_phys_page_attrs_debug(CPUState *cpu, 
vaddr addr,
 MemTxAttrs *attrs);
 int mb_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int mb_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+int mb_cpu_gdb_read_stack_protect(CPUArchState *cpu, GByteArray *buf, int reg);
+int mb_cpu_gdb_write_stack_protect(CPUArchState *cpu, uint8_t *buf, int reg);
 
 static inline uint32_t mb_cpu_read_msr(const CPUMBState *env)
 {
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 817681f9b2..a2d2f5c340 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -28,6 +28,7 @@
 #include "qemu/module.h"
 #include "hw/qdev-properties.h"
 #include "exec/exec-all.h"
+#include "exec/gdbstub.h"
 #include "fpu/softfloat-helpers.h"
 
 static const struct {
@@ -294,6 +295,9 @@ static void mb_cpu_initfn(Object *obj)
 CPUMBState *env = >env;
 
 cpu_set_cpustate_pointers(cpu);
+gdb_register_coprocessor(CPU(cpu), mb_cpu_gdb_read_stack_protect,
+ mb_cpu_gdb_write_stack_protect, 2,
+ "microblaze-stack-protect.xml", 0);
 
 set_float_rounding_mode(float_round_nearest_even, >fp_status);
 
@@ -422,7 +426,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
 cc->sysemu_ops = _sysemu_ops;
 #endif
 device_class_set_props(dc, mb_properties);
-cc->gdb_num_core_regs = 32 + 27;
+cc->gdb_num_core_regs = 32 + 25;
+cc->gdb_core_xml_file = "microblaze-core.xml";
 
 cc->disas_set_info = mb_disas_set_info;
 cc->tcg_ops = _tcg_ops;
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index 2e6e070051..8143fcae88 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -39,8 +39,11 @@ enum {
 GDB_PVR0  = 32 + 6,
 GDB_PVR11 = 32 + 17,
 GDB_EDR   = 32 + 18,
-GDB_SLR   = 32 + 25,
-GDB_SHR   = 32 + 26,
+};
+
+enum {
+GDB_SP_SHL,
+GDB_SP_SHR,
 };
 
 int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
@@ -83,12 +86,6 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray 
*mem_buf, int n)
 case GDB_EDR:
 val = env->edr;
 break;
-case GDB_SLR:
-val = env->slr;
-break;
-case GDB_SHR:
-val = 

[PULL 2/7] linux-user/sparc: Raise SIGILL for all unhandled software traps

[Richard Henderson]

The linux kernel's trap tables vector all unassigned trap
numbers to BAD_TRAP, which then raises SIGILL.

Tested-by: Ilya Leoshkevich 
Reported-by: Ilya Leoshkevich 
Signed-off-by: Richard Henderson 
---
 linux-user/sparc/cpu_loop.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
index 434c90a55f..c120c42278 100644
--- a/linux-user/sparc/cpu_loop.c
+++ b/linux-user/sparc/cpu_loop.c
@@ -248,6 +248,14 @@ void cpu_loop (CPUSPARCState *env)
 cpu_exec_step_atomic(cs);
 break;
 default:
+/*
+ * Most software trap numbers vector to BAD_TRAP.
+ * Handle anything not explicitly matched above.
+ */
+if (trapnr >= TT_TRAP && trapnr <= TT_TRAP + 0x7f) {
+force_sig_fault(TARGET_SIGILL, ILL_ILLTRP, env->pc);
+break;
+}
 fprintf(stderr, "Unhandled trap: 0x%x\n", trapnr);
 cpu_dump_state(cs, stderr, 0);
 exit(EXIT_FAILURE);
-- 
2.34.1

[PULL 5/7] linux-user/microblaze: Handle privileged exception

[Richard Henderson]

From: Ilya Leoshkevich 

Follow what kernel's full_exception() is doing.

Reviewed-by: Richard Henderson 
Signed-off-by: Ilya Leoshkevich 
Message-Id: <20230214140829.45392-4-...@linux.ibm.com>
Signed-off-by: Richard Henderson 
---
 linux-user/microblaze/cpu_loop.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/linux-user/microblaze/cpu_loop.c b/linux-user/microblaze/cpu_loop.c
index 5ccf9e942e..212e62d0a6 100644
--- a/linux-user/microblaze/cpu_loop.c
+++ b/linux-user/microblaze/cpu_loop.c
@@ -25,8 +25,8 @@
 
 void cpu_loop(CPUMBState *env)
 {
+int trapnr, ret, si_code, sig;
 CPUState *cs = env_cpu(env);
-int trapnr, ret, si_code;
 
 while (1) {
 cpu_exec_start(cs);
@@ -76,6 +76,7 @@ void cpu_loop(CPUMBState *env)
 env->iflags &= ~(IMM_FLAG | D_FLAG);
 switch (env->esr & 31) {
 case ESR_EC_DIVZERO:
+sig = TARGET_SIGFPE;
 si_code = TARGET_FPE_INTDIV;
 break;
 case ESR_EC_FPU:
@@ -84,6 +85,7 @@ void cpu_loop(CPUMBState *env)
  * if there's no recognized bit set.  Possibly this
  * implies that si_code is 0, but follow the structure.
  */
+sig = TARGET_SIGFPE;
 si_code = env->fsr;
 if (si_code & FSR_IO) {
 si_code = TARGET_FPE_FLTINV;
@@ -97,13 +99,17 @@ void cpu_loop(CPUMBState *env)
 si_code = TARGET_FPE_FLTRES;
 }
 break;
+case ESR_EC_PRIVINSN:
+sig = SIGILL;
+si_code = ILL_PRVOPC;
+break;
 default:
 fprintf(stderr, "Unhandled hw-exception: 0x%x\n",
 env->esr & ESR_EC_MASK);
 cpu_dump_state(cs, stderr, 0);
 exit(EXIT_FAILURE);
 }
-force_sig_fault(TARGET_SIGFPE, si_code, env->pc);
+force_sig_fault(sig, si_code, env->pc);
 break;
 
 case EXCP_DEBUG:
-- 
2.34.1

[PULL 4/7] cpus: Make {start,end}_exclusive() recursive

[Richard Henderson]

From: Ilya Leoshkevich 

Currently dying to one of the core_dump_signal()s deadlocks, because
dump_core_and_abort() calls start_exclusive() two times: first via
stop_all_tasks(), and then via preexit_cleanup() ->
qemu_plugin_user_exit().

There are a number of ways to solve this: resume after dumping core;
check cpu_in_exclusive_context() in qemu_plugin_user_exit(); or make
{start,end}_exclusive() recursive. Pick the last option, since it's
the most straightforward one.

Fixes: da91c1920242 ("linux-user: Clean up when exiting due to a signal")
Reviewed-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Signed-off-by: Ilya Leoshkevich 
Message-Id: <20230214140829.45392-3-...@linux.ibm.com>
Signed-off-by: Richard Henderson 
---
 include/hw/core/cpu.h |  4 ++--
 cpus-common.c | 12 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 2417597236..671f041bec 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -349,7 +349,7 @@ struct CPUState {
 bool unplug;
 bool crash_occurred;
 bool exit_request;
-bool in_exclusive_context;
+int exclusive_context_count;
 uint32_t cflags_next_tb;
 /* updates protected by BQL */
 uint32_t interrupt_request;
@@ -758,7 +758,7 @@ void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func 
func, run_on_cpu_data
  */
 static inline bool cpu_in_exclusive_context(const CPUState *cpu)
 {
-return cpu->in_exclusive_context;
+return cpu->exclusive_context_count;
 }
 
 /**
diff --git a/cpus-common.c b/cpus-common.c
index 793364dc0e..39f355de98 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -192,6 +192,11 @@ void start_exclusive(void)
 CPUState *other_cpu;
 int running_cpus;
 
+if (current_cpu->exclusive_context_count) {
+current_cpu->exclusive_context_count++;
+return;
+}
+
 qemu_mutex_lock(_cpu_list_lock);
 exclusive_idle();
 
@@ -219,13 +224,16 @@ void start_exclusive(void)
  */
 qemu_mutex_unlock(_cpu_list_lock);
 
-current_cpu->in_exclusive_context = true;
+current_cpu->exclusive_context_count = 1;
 }
 
 /* Finish an exclusive operation.  */
 void end_exclusive(void)
 {
-current_cpu->in_exclusive_context = false;
+current_cpu->exclusive_context_count--;
+if (current_cpu->exclusive_context_count) {
+return;
+}
 
 qemu_mutex_lock(_cpu_list_lock);
 qatomic_set(_cpus, 0);
-- 
2.34.1

Re: [RFC PATCH 0/2] Add flag as THP allocation hint for memfd_restricted() syscall

[Yuan Yao]

On Sat, Feb 18, 2023 at 12:43:00AM +, Ackerley Tng wrote:
> Hello,
>
> This patchset builds upon the memfd_restricted() system call that has
> been discussed in the ‘KVM: mm: fd-based approach for supporting KVM’
> patch series, at
> https://lore.kernel.org/lkml/20221202061347.1070246-1-chao.p.p...@linux.intel.com/T/#m7e944d7892afdd1d62a03a287bd488c56e377b0c
>
> The tree can be found at:
> https://github.com/googleprodkernel/linux-cc/tree/restrictedmem-rmfd-hugepage
>
> Following the RFC to provide mount for memfd_restricted() syscall at
> https://lore.kernel.org/lkml/cover.1676507663.git.ackerley...@google.com/T/#u,
> this patchset adds the RMFD_HUGEPAGE flag to the memfd_restricted()
> syscall, which will hint the kernel to use Transparent HugePages to
> back restrictedmem pages.
>
> This supplements the interface proposed earlier, which requires the
> creation of a tmpfs mount to be passed to memfd_restricted(), with a
> more direct per-file hint.
>
> Dependencies:
>
> + Sean’s iteration of the ‘KVM: mm: fd-based approach for supporting
>   KVM’ patch series at
>   https://github.com/sean-jc/linux/tree/x86/upm_base_support
> + Proposed fix for restrictedmem_getattr() as mentioned on the mailing
>   list at
>   
> https://lore.kernel.org/lkml/diqzzga0fv96@ackerleytng-cloudtop-sg.c.googlers.com/
> + Hugh’s patch:
>   
> https://lore.kernel.org/lkml/c140f56a-1aa3-f7ae-b7d1-93da7d5a3...@google.com/,
>   which provides functionality in shmem that reads the VM_HUGEPAGE
>   flag in key functions shmem_is_huge() and shmem_get_inode()

Will Hugh's patch be merged into 6.3 ? I didn't find it in 6.2-rc8.
IMHO this patch won't work without Hugh's patch, or at least need
another way, e.g. HMEM_SB(inode->i_sb)->huge.

>
> Future work/TODOs:
> + man page for the memfd_restricted() syscall
> + Support for per file NUMA binding hints
>
> Ackerley Tng (2):
>   mm: restrictedmem: Add flag as THP allocation hint for
> memfd_restricted() syscall
>   selftests: restrictedmem: Add selftest for RMFD_HUGEPAGE
>
>  include/uapi/linux/restrictedmem.h|  1 +
>  mm/restrictedmem.c| 27 ---
>  .../restrictedmem_hugepage_test.c | 25 +
>  3 files changed, 43 insertions(+), 10 deletions(-)
>
> --
> 2.39.2.637.g21b0678d19-goog
>

Re: [PATCH RESEND 18/18] i386: Add new property to control L2 cache topo in CPUID.04H

[Zhao Liu]

On Fri, Feb 17, 2023 at 11:45:58AM +0800, wangyanan (Y) wrote:
> Date: Fri, 17 Feb 2023 11:45:58 +0800
> From: "wangyanan (Y)" 
> Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
>  cache topo in CPUID.04H
> 
> 在 2023/2/17 11:35, Zhao Liu 写道:
> > On Thu, Feb 16, 2023 at 09:14:54PM +0800, wangyanan (Y) wrote:
> > > Date: Thu, 16 Feb 2023 21:14:54 +0800
> > > From: "wangyanan (Y)" 
> > > Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
> > >   cache topo in CPUID.04H
> > > 
> > > 在 2023/2/13 17:36, Zhao Liu 写道:
> > > > From: Zhao Liu 
> > > > 
> > > > The property x-l2-cache-topo will be used to change the L2 cache
> > > > topology in CPUID.04H.
> > > > 
> > > > Now it allows user to set the L2 cache is shared in core level or
> > > > cluster level.
> > > > 
> > > > If user passes "-cpu x-l2-cache-topo=[core|cluster]" then older L2 cache
> > > > topology will be overrided by the new topology setting.
> > > Currently x-l2-cache-topo only defines the share level *globally*.
> > Yes, will set for all CPUs.
> > 
> > > I'm thinking how we can make the property more powerful so that it
> > > can specify which CPUs share l2 on core level and which CPUs share
> > > l2 on cluster level.
> > > 
> > > What would Intel's Hybrid CPUs do? Determine the l2 share level
> > > is core or cluster according to the CPU core type (Atom or Core)?
> > > While ARM does not have the core type concept but have CPUs
> > > that l2 is shared on different levels in the same system.
> > For example, Alderlake's "core" shares 1 L2 per core and every 4 "atom"s
> > share 1 L2. For this case, we can set the topology as:
> > 
> > cluster0 has 1 "core" and cluster1 has 4 "atom". Then set L2 shared on
> > cluster level.
> > 
> > Since cluster0 has only 1 "core" type core, then L2 per "core" works.
> This brings restriction to the users that cluster0 must have *1* core-type
> core.
> What if we set 2 vCores in cluster0 and 4 vCores in cluster1,  and bind
> cores in
> cluster0 to 2 core-type pCores and bind cores in cluster1 to 4 atom-type
> pCores？I think this is a necessary use case too.

At present, the cache topology level and core type are not bound, so the
cache topology level can also be adjusted for any vCores.

> > Not sure if this idea can be applied to arm?
> > 
> > > Thanks,
> > > Yanan
> > > > Here we expose to user "cluster" instead of "module", to be consistent
> > > > with "cluster-id" naming.
> > > > 
> > > > Since CPUID.04H is used by intel CPUs, this property is available on
> > > > intel CPUs as for now.
> > > > 
> > > > When necessary, it can be extended to CPUID.801DH for amd CPUs.
> > > > 
> > > > Signed-off-by: Zhao Liu 
> > > > ---
> > > >target/i386/cpu.c | 33 -
> > > >target/i386/cpu.h |  2 ++
> > > >2 files changed, 34 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > index 5816dc99b1d4..cf84c720a431 100644
> > > > --- a/target/i386/cpu.c
> > > > +++ b/target/i386/cpu.c
> > > > @@ -240,12 +240,15 @@ static uint32_t 
> > > > max_processor_ids_for_cache(CPUCacheInfo *cache,
> > > >case CORE:
> > > >num_ids = 1 << apicid_core_offset(topo_info);
> > > >break;
> > > > +case MODULE:
> > > > +num_ids = 1 << apicid_module_offset(topo_info);
> > > > +break;
> > > >case DIE:
> > > >num_ids = 1 << apicid_die_offset(topo_info);
> > > >break;
> > > >default:
> > > >/*
> > > > - * Currently there is no use case for SMT, MODULE and PACKAGE, 
> > > > so use
> > > > + * Currently there is no use case for SMT and PACKAGE, so use
> > > > * assert directly to facilitate debugging.
> > > > */
> > > >g_assert_not_reached();
> > > > @@ -6633,6 +6636,33 @@ static void x86_cpu_realizefn(DeviceState *dev, 
> > > > Error **errp)
> > > >env->cache_info_amd.l3_cache = _l3_cache;
> > > >}
> > > > +if (cpu->l2_cache_topo_level) {
> > > > +/*
> > > > + * FIXME: Currently only supports changing CPUID[4] (for 
> > > > intel), and
> > > > + * will support changing CPUID[0x801D] when necessary.
> > > > + */
> > > > +if (!IS_INTEL_CPU(env)) {
> > > > +error_setg(errp, "only intel cpus supports 
> > > > x-l2-cache-topo");
> > > > +return;
> > > > +}
> > > > +
> > > > +if (!strcmp(cpu->l2_cache_topo_level, "core")) {
> > > > +env->cache_info_cpuid4.l2_cache->share_level = CORE;
> > > > +} else if (!strcmp(cpu->l2_cache_topo_level, "cluster")) {
> > > > +/*
> > > > + * We expose to users "cluster" instead of "module", to be
> > > > + * consistent with "cluster-id" naming.
> > > > + */
> > > > +env->cache_info_cpuid4.l2_cache->share_level = MODULE;
> > > > +} else {
> > 

Re: [PATCH RESEND 18/18] i386: Add new property to control L2 cache topo in CPUID.04H

[Zhao Liu]

On Fri, Feb 17, 2023 at 05:08:31PM +0800, wangyanan (Y) wrote:
> Date: Fri, 17 Feb 2023 17:08:31 +0800
> From: "wangyanan (Y)" 
> Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
>  cache topo in CPUID.04H
> 
> 在 2023/2/17 15:26, Zhao Liu 写道:
> > On Fri, Feb 17, 2023 at 12:07:01PM +0800, wangyanan (Y) wrote:
> > > Date: Fri, 17 Feb 2023 12:07:01 +0800
> > > From: "wangyanan (Y)" 
> > > Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
> > >   cache topo in CPUID.04H
> > > 
> > > 在 2023/2/17 11:35, Zhao Liu 写道:
> > > > On Thu, Feb 16, 2023 at 09:14:54PM +0800, wangyanan (Y) wrote:
> > > > > Date: Thu, 16 Feb 2023 21:14:54 +0800
> > > > > From: "wangyanan (Y)" 
> > > > > Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
> > > > >cache topo in CPUID.04H
> > > > > 
> > > > > 在 2023/2/13 17:36, Zhao Liu 写道:
> > > > > > From: Zhao Liu 
> > > > > > 
> > > > > > The property x-l2-cache-topo will be used to change the L2 cache
> > > > > > topology in CPUID.04H.
> > > > > > 
> > > > > > Now it allows user to set the L2 cache is shared in core level or
> > > > > > cluster level.
> > > > > > 
> > > > > > If user passes "-cpu x-l2-cache-topo=[core|cluster]" then older L2 
> > > > > > cache
> > > > > > topology will be overrided by the new topology setting.
> > > > > Currently x-l2-cache-topo only defines the share level *globally*.
> > > > Yes, will set for all CPUs.
> > > > 
> > > > > I'm thinking how we can make the property more powerful so that it
> > > > > can specify which CPUs share l2 on core level and which CPUs share
> > > > > l2 on cluster level.
> > > > > 
> > > > > What would Intel's Hybrid CPUs do? Determine the l2 share level
> > > > > is core or cluster according to the CPU core type (Atom or Core)?
> > > > > While ARM does not have the core type concept but have CPUs
> > > > > that l2 is shared on different levels in the same system.
> > > > For example, Alderlake's "core" shares 1 L2 per core and every 4 "atom"s
> > > > share 1 L2. For this case, we can set the topology as:
> > > > 
> > > > cluster0 has 1 "core" and cluster1 has 4 "atom". Then set L2 shared on
> > > > cluster level.
> > > > 
> > > > Since cluster0 has only 1 "core" type core, then L2 per "core" works.
> > > > 
> > > > Not sure if this idea can be applied to arm?
> > > For a CPU topopoly where we have 2 clusters totally, 2 cores in cluster0
> > > have their own L1/L2 cache and 2 threads in each core, 4 cores in cluster1
> > > share one L2 cache and 1 thread in each core. The global way does not
> > > work well.
> > > 
> > > What about defining something general, which looks like -numa config:
> > > -cache-topo cache=l2, share_level="core", cpus='0-3'
> > > -cache-topo cache=l2, share_level="cluster", cpus='4-7'
> > Hi Yanan, here it may be necessary to check whether the cpu index set
> > in "cpus" is reasonable through the specific cpu topology.
> Yes, the validity of the cache topo configs from the users should be
> check in machine_parse_cache_topo ( if we will have this func).
> It's not a big deal, we always need the validity checks.

I guess that verification needs to build up the full cpu topology, as
done in another hybrid RFC. So, should the cpu-topology.h related
patches in that RFC be split out and sent first?

> 
> In summary:
> 1、There can not be the same cpus in different "cpus" list.
> 2、A combination of all the "cpus" list should *just* cover all the CPUs
> in the machine
> 3、Most importantly, cpus in the same cluster must be set with the
> same cache "share_level" (core or cluster) and cpus in the same core
> must also be set with the same cache "share_level".

Got it, thx.

> > For example, core0 has 2 CPUs: cpu0 and cpu1, and core1 has 2 CPUs: cpu2
> > and cpu3, then set l2 as:
> > 
> > -cache-topo cache=l2, share_level="core", cpus='0-2'
> > -cache-topo cache=l2, share_level="core", cpus='3'
> > 
> > Whether this command is legal depends on the meaning we give to the
> > parameter "cpu":
> s/cpus/cpu.
> It means all the afftected CPUs, e.g, the second case.
> > 1. If "cpu" means all cpus share the cache set in this command, then
> > this command should fail since cpu2 and cpu3 are in a core.
> > 
> > 2. If "cpu" means the affected cpus, then this command should find the
> > cores they belong to according to the cpu topology, and set L2 for those
> > cores. This command may return success.
> > 
> > What about removing share_level and ask "cpu" to mean all the sharing
> > cpus to avoid checking the cpu topology?
> > 
> > Then the above example should be:
> > 
> > -cache-topo cache=l2, cpus='0-1'
> > -cache-topo cache=l2, cpus='2-3'
> Sorry, I dont understand how we will know the cache share_level of
> cpus '0-1' or '2-3'. What will the CLIs will be like if we change the
> belows CLIs by removing the "share_level" params.
> 
> -cache-topo cache=l2, share_level="core", cpus='0-3'
> -cache-topo cache=l2, share_level="cluster", cpus='4-7'
> > This 

Optimization for the virtio-balloon feature on the ARM platform

[Yangming via]

Dear QEMU maintainers,

I am writing to discuss a possible optimization for the virtio-balloon feature 
on the ARM platform. The 'virtio_balloon_set_config' function is called 
frequently during the balloon inflation process, and its subfunction 
'get_current_ram_size' needs to traverse the virtual machine's memory modules 
in order to count the current virtual machine's memory (i.e initial ram size + 
hotplugged memory). This can be very time consuming on the ARM platform, as the 
ARM virtual machine has much more complex memory modules than the x86 virtual 
machine.

Therefore, I suggest introducing a global variable, 'total_ram_size', that 
would be updated only when the balloon is initialized and hotplug memory has 
completed. This would increase the efficiency of balloon inflation by more than 
60% on the ARM platform.

The following code is part of the optimization for balloon:

--- a/qemu/hw/virtio/virtio-balloon.c
+++ b/qemu/hw/virtio/virtio-balloon.c
static void virtio_balloon_set_config(...)
...
-ram_addr_t vm_ram_size = get_current_ram_size();
+   ram_addr_t vm_ram_size = total_ram_size;
...
I hope this suggestion could be considered or discussed by QEMU developers. I 
would love to seeing this improvement added to QEMU in the future.

Best regards,
Qi

Re: [PATCH v1 1/2] hw: allwinner-i2c: Make the trace message more readable

[qianfan]

在 2023/2/20 6:30, Philippe Mathieu-Daudé 写道:

Hi,

On 17/2/23 10:42, qianfangui...@163.com wrote:

From: qianfan Zhao 

Next is an example when allwinner_i2c_rw enabled:

allwinner_i2c_rw write   CNTR[0x0c]: 50 { M_STP BUS_EN  }
allwinner_i2c_rw write   CNTR[0x0c]: e4 { A_ACK M_STA BUS_EN INT_EN  }
allwinner_i2c_rw  read   CNTR[0x0c]: cc { A_ACK INT_FLAG BUS_EN INT_EN }
allwinner_i2c_rw  read   STAT[0x10]: 08 { STAT_M_STA_TX }

Signed-off-by: qianfan Zhao 
---
  hw/i2c/allwinner-i2c.c | 90 +-
  hw/i2c/trace-events    |  4 +-
  2 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c
index a435965836..36b387520f 100644
--- a/hw/i2c/allwinner-i2c.c
+++ b/hw/i2c/allwinner-i2c.c
@@ -129,6 +129,39 @@ enum {
  STAT_IDLE = 0x1f
  } TWI_STAT_STA;
  +#define TWI_STAT_STA_DESC(sta)  [sta] = #sta
+static const char *twi_stat_sta_descriptors[] = {
+    TWI_STAT_STA_DESC(STAT_BUS_ERROR),
+    TWI_STAT_STA_DESC(STAT_M_STA_TX),
+    TWI_STAT_STA_DESC(STAT_M_RSTA_TX),
+    TWI_STAT_STA_DESC(STAT_M_ADDR_WR_ACK),
+    TWI_STAT_STA_DESC(STAT_M_ADDR_WR_NACK),
+    TWI_STAT_STA_DESC(STAT_M_DATA_TX_ACK),
+    TWI_STAT_STA_DESC(STAT_M_DATA_TX_NACK),
+    TWI_STAT_STA_DESC(STAT_M_ARB_LOST),
+    TWI_STAT_STA_DESC(STAT_M_ADDR_RD_ACK),
+    TWI_STAT_STA_DESC(STAT_M_ADDR_RD_NACK),
+    TWI_STAT_STA_DESC(STAT_M_DATA_RX_ACK),
+    TWI_STAT_STA_DESC(STAT_M_DATA_RX_NACK),
+    TWI_STAT_STA_DESC(STAT_S_ADDR_WR_ACK),
+    TWI_STAT_STA_DESC(STAT_S_ARB_LOST_AW_ACK),
+    TWI_STAT_STA_DESC(STAT_S_GCA_ACK),
+    TWI_STAT_STA_DESC(STAT_S_ARB_LOST_GCA_ACK),
+    TWI_STAT_STA_DESC(STAT_S_DATA_RX_SA_ACK),
+    TWI_STAT_STA_DESC(STAT_S_DATA_RX_SA_NACK),
+    TWI_STAT_STA_DESC(STAT_S_DATA_RX_GCA_ACK),
+    TWI_STAT_STA_DESC(STAT_S_DATA_RX_GCA_NACK),
+    TWI_STAT_STA_DESC(STAT_S_STP_RSTA),
+    TWI_STAT_STA_DESC(STAT_S_ADDR_RD_ACK),
+    TWI_STAT_STA_DESC(STAT_S_ARB_LOST_AR_ACK),
+    TWI_STAT_STA_DESC(STAT_S_DATA_TX_ACK),
+    TWI_STAT_STA_DESC(STAT_S_DATA_TX_NACK),
+    TWI_STAT_STA_DESC(STAT_S_LB_TX_ACK),
+    TWI_STAT_STA_DESC(STAT_M_2ND_ADDR_WR_ACK),
+    TWI_STAT_STA_DESC(STAT_M_2ND_ADDR_WR_NACK),
+    TWI_STAT_STA_DESC(STAT_IDLE),
+};
+
  static const char *allwinner_i2c_get_regname(unsigned offset)
  {
  switch (offset) {
@@ -155,6 +188,59 @@ static const char 
*allwinner_i2c_get_regname(unsigned offset)

  }
  }
  +static const char *twi_cntr_reg_bits[] = {
+    [2] = "A_ACK",
+    [3] = "INT_FLAG",
+    [4] = "M_STP",
+    [5] = "M_STA",
+    [6] = "BUS_EN",
+    [7] = "INT_EN",
+};
+
+static void trace_buffer_append_bit_descriptors(char *s, size_t sz,
+    unsigned int value,
+    unsigned int start,
+    unsigned int end,
+    const char 
**desc_arrays)

+{
+    for (; start <= end; start++) {


You call this once, so no need to pass a desc_arrays[] argument.
Directly iterate over twi_cntr_reg_bits[] and its ARRAY_SIZE().


create desc_arrays is useful if there has more register need dump. such as
trace_buffer_append_bit_descriptors(..., twi_cntr_reg_bits)
or trace_buffer_append_bit_descriptors(..., twi_line_cntr_reg_bits)




+    if (value & (1 << start)) {
+    strncat(s, desc_arrays[start], sz - 1);


Watch out, desc_arrays[start] could be NULL.


if ((value & (1 << start)) && desc_arrays[start]) is better.




+    strncat(s, " ", sz - 1);
+    }
+    }
+}
+
+static void allwinner_i2c_trace_rw(int is_write, unsigned int offset,


Please use 'bool' for 'is_write' which is a boolean.


+ unsigned int value)
+{


You can call trace_event_get_state_backends() to check if a
trace event is enabled and return early without further processing.


got it.




+    char s[256] = { 0 };
+
+    snprintf(s, sizeof(s), "%s %6s[0x%02x]: %02x ",


Please prefix hexadecimal values with 0x.


OK.


+ is_write ? "write": " read",
+ allwinner_i2c_get_regname(offset), offset,
+ value);


We prefer the safer g_autofree ... g_strdup_printf().


The next trace_buffer_append_bit_descriptors will appending to a pre-alloced 
buffer,
so I create a buffer. Total 256 bytes seems enough for the trace strings.




+    switch (offset) {
+    case TWI_CNTR_REG:
+    strncat(s, "{ ", sizeof(s) - 1);
+    trace_buffer_append_bit_descriptors(s, sizeof(s), value,
+    2, 7, twi_cntr_reg_bits);
+    strncat(s, " }", sizeof(s) - 1);
+    break;
+    case TWI_STAT_REG:
+    if (STAT_TO_STA(value) <= STAT_IDLE) {
+    strncat(s, "{ ", sizeof(s) - 1);
+    strncat(s, twi_stat_sta_descriptors[STAT_TO_STA(value)],
+    sizeof(s) - 1);
+    strncat(s, " }", sizeof(s) - 1);
+    }
+    break;
+    }
+
+    

Re: [PATCH v1 1/2] hw: allwinner-i2c: Make the trace message more readable

[Philippe Mathieu-Daudé]

Hi,

On 17/2/23 10:42, qianfangui...@163.com wrote:

From: qianfan Zhao 

Next is an example when allwinner_i2c_rw enabled:

allwinner_i2c_rw write   CNTR[0x0c]: 50 { M_STP BUS_EN  }
allwinner_i2c_rw write   CNTR[0x0c]: e4 { A_ACK M_STA BUS_EN INT_EN  }
allwinner_i2c_rw  read   CNTR[0x0c]: cc { A_ACK INT_FLAG BUS_EN INT_EN }
allwinner_i2c_rw  read   STAT[0x10]: 08 { STAT_M_STA_TX }

Signed-off-by: qianfan Zhao 
---
  hw/i2c/allwinner-i2c.c | 90 +-
  hw/i2c/trace-events|  4 +-
  2 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c
index a435965836..36b387520f 100644
--- a/hw/i2c/allwinner-i2c.c
+++ b/hw/i2c/allwinner-i2c.c
@@ -129,6 +129,39 @@ enum {
  STAT_IDLE = 0x1f
  } TWI_STAT_STA;
  
+#define TWI_STAT_STA_DESC(sta)  [sta] = #sta

+static const char *twi_stat_sta_descriptors[] = {
+TWI_STAT_STA_DESC(STAT_BUS_ERROR),
+TWI_STAT_STA_DESC(STAT_M_STA_TX),
+TWI_STAT_STA_DESC(STAT_M_RSTA_TX),
+TWI_STAT_STA_DESC(STAT_M_ADDR_WR_ACK),
+TWI_STAT_STA_DESC(STAT_M_ADDR_WR_NACK),
+TWI_STAT_STA_DESC(STAT_M_DATA_TX_ACK),
+TWI_STAT_STA_DESC(STAT_M_DATA_TX_NACK),
+TWI_STAT_STA_DESC(STAT_M_ARB_LOST),
+TWI_STAT_STA_DESC(STAT_M_ADDR_RD_ACK),
+TWI_STAT_STA_DESC(STAT_M_ADDR_RD_NACK),
+TWI_STAT_STA_DESC(STAT_M_DATA_RX_ACK),
+TWI_STAT_STA_DESC(STAT_M_DATA_RX_NACK),
+TWI_STAT_STA_DESC(STAT_S_ADDR_WR_ACK),
+TWI_STAT_STA_DESC(STAT_S_ARB_LOST_AW_ACK),
+TWI_STAT_STA_DESC(STAT_S_GCA_ACK),
+TWI_STAT_STA_DESC(STAT_S_ARB_LOST_GCA_ACK),
+TWI_STAT_STA_DESC(STAT_S_DATA_RX_SA_ACK),
+TWI_STAT_STA_DESC(STAT_S_DATA_RX_SA_NACK),
+TWI_STAT_STA_DESC(STAT_S_DATA_RX_GCA_ACK),
+TWI_STAT_STA_DESC(STAT_S_DATA_RX_GCA_NACK),
+TWI_STAT_STA_DESC(STAT_S_STP_RSTA),
+TWI_STAT_STA_DESC(STAT_S_ADDR_RD_ACK),
+TWI_STAT_STA_DESC(STAT_S_ARB_LOST_AR_ACK),
+TWI_STAT_STA_DESC(STAT_S_DATA_TX_ACK),
+TWI_STAT_STA_DESC(STAT_S_DATA_TX_NACK),
+TWI_STAT_STA_DESC(STAT_S_LB_TX_ACK),
+TWI_STAT_STA_DESC(STAT_M_2ND_ADDR_WR_ACK),
+TWI_STAT_STA_DESC(STAT_M_2ND_ADDR_WR_NACK),
+TWI_STAT_STA_DESC(STAT_IDLE),
+};
+
  static const char *allwinner_i2c_get_regname(unsigned offset)
  {
  switch (offset) {
@@ -155,6 +188,59 @@ static const char *allwinner_i2c_get_regname(unsigned 
offset)
  }
  }
  
+static const char *twi_cntr_reg_bits[] = {

+[2] = "A_ACK",
+[3] = "INT_FLAG",
+[4] = "M_STP",
+[5] = "M_STA",
+[6] = "BUS_EN",
+[7] = "INT_EN",
+};
+
+static void trace_buffer_append_bit_descriptors(char *s, size_t sz,
+unsigned int value,
+unsigned int start,
+unsigned int end,
+const char **desc_arrays)
+{
+for (; start <= end; start++) {


You call this once, so no need to pass a desc_arrays[] argument.
Directly iterate over twi_cntr_reg_bits[] and its ARRAY_SIZE().


+if (value & (1 << start)) {
+strncat(s, desc_arrays[start], sz - 1);


Watch out, desc_arrays[start] could be NULL.


+strncat(s, " ", sz - 1);
+}
+}
+}
+
+static void allwinner_i2c_trace_rw(int is_write, unsigned int offset,


Please use 'bool' for 'is_write' which is a boolean.


+   unsigned int value)
+{


You can call trace_event_get_state_backends() to check if a
trace event is enabled and return early without further processing.


+char s[256] = { 0 };
+
+snprintf(s, sizeof(s), "%s %6s[0x%02x]: %02x ",


Please prefix hexadecimal values with 0x.


+ is_write ? "write": " read",
+ allwinner_i2c_get_regname(offset), offset,
+ value);


We prefer the safer g_autofree ... g_strdup_printf().


+switch (offset) {
+case TWI_CNTR_REG:
+strncat(s, "{ ", sizeof(s) - 1);
+trace_buffer_append_bit_descriptors(s, sizeof(s), value,
+2, 7, twi_cntr_reg_bits);
+strncat(s, " }", sizeof(s) - 1);
+break;
+case TWI_STAT_REG:
+if (STAT_TO_STA(value) <= STAT_IDLE) {
+strncat(s, "{ ", sizeof(s) - 1);
+strncat(s, twi_stat_sta_descriptors[STAT_TO_STA(value)],
+sizeof(s) - 1);
+strncat(s, " }", sizeof(s) - 1);
+}
+break;
+}
+
+trace_allwinner_i2c_rw(s);
+}
+
  static inline bool allwinner_i2c_is_reset(AWI2CState *s)
  {
  return s->srst & TWI_SRST_MASK;
@@ -271,7 +357,7 @@ static uint64_t allwinner_i2c_read(void *opaque, hwaddr 
offset,
  break;
  }
  
-trace_allwinner_i2c_read(allwinner_i2c_get_regname(offset), offset, value);

+allwinner_i2c_trace_rw(0, (unsigned int)offset, (unsigned int)value);
  
  return (uint64_t)value;

  }
@@ -283,7 +369,7 @@ static void 

Re: [PATCH v2 07/18] hw/ide/piix: Ensure IDE output IRQs are wired at realization

[Philippe Mathieu-Daudé]

+Daniel, Igor, Marcel & libvirt

On 16/2/23 16:33, Philippe Mathieu-Daudé wrote:

On 16/2/23 15:43, Bernhard Beschow wrote:



On Wed, Feb 15, 2023 at 5:20 PM Philippe Mathieu-Daudé 
mailto:phi...@linaro.org>> wrote:


    Ensure both IDE output IRQ lines are wired.

    We can remove the last use of isa_get_irq(NULL).

    Signed-off-by: Philippe Mathieu-Daudé mailto:phi...@linaro.org>>
    ---
  hw/ide/piix.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)

    diff --git a/hw/ide/piix.c b/hw/ide/piix.c
    index 9d876dd4a7..b75a4ddcca 100644
    --- a/hw/ide/piix.c
    +++ b/hw/ide/piix.c
    @@ -133,14 +133,17 @@ static bool pci_piix_init_bus(PCIIDEState *d,
    unsigned i, Error **errp)
      static const struct {
          int iobase;
          int iobase2;
    -        int isairq;
      } port_info[] = {
    -        {0x1f0, 0x3f6, 14},
    -        {0x170, 0x376, 15},
    +        {0x1f0, 0x3f6},
    +        {0x170, 0x376},
      };
      int ret;

    -    qemu_irq irq_out = d->irq[i] ? : isa_get_irq(NULL,
    port_info[i].isairq);
    +    if (!d->irq[i]) {
    +        error_setg(errp, "output IDE IRQ %u not connected", i);
    +        return false;
    +    }
    +
      ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
      ret = ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
                            port_info[i].iobase2);
    @@ -149,7 +152,7 @@ static bool pci_piix_init_bus(PCIIDEState *d,
    unsigned i, Error **errp)
                           object_get_typename(OBJECT(d)), i);
          return false;
      }
    -    ide_bus_init_output_irq(>bus[i], irq_out);
    +    ide_bus_init_output_irq(>bus[i], d->irq[i]);

      bmdma_init(>bus[i], >bmdma[i], d);
      d->bmdma[i].bus = >bus[i];
    --     2.38.1


This now breaks user-created  piix3-ide:

   qemu-system-x86_64 -M q35 -device piix3-ide

Results in:

   qemu-system-x86_64: -device piix3-ide: output IDE IRQ 0 not connected


Thank you for this real-life-impossible-but-exists-in-QEMU test-case!


Do we really want to maintain this Frankenstein use case?

1/ Q35 comes with a PCIe bus on which sits a ICH chipset, which already
   contains a AHCI function exposing multiple IDE buses.
   What is the point on using an older tech?

2/ Why can we plug a PCI function on a PCIe bridge without using a
   pcie-pci-bridge?

3/ Chipsets come as a whole. Software drivers might expect all PCI
   functions working (Linux considering each function individually
   is not the norm)


I get your use case working with the following diff [*]:

-- >8 --
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 74e2f4288d..cb1628963a 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -140,8 +140,19 @@ static bool pci_piix_init_bus(PCIIDEState *d, 
unsigned i, Error **errp)

 };

 if (!d->irq[i]) {
-error_setg(errp, "output IDE IRQ %u not connected", i);
-return false;
+if (DEVICE_GET_CLASS(d)->user_creatable) {
+/* Returns NULL unless there is exactly one ISA bus */
+Object *isabus = object_resolve_path_type("", TYPE_ISA_BUS, 
NULL);

+
+if (!isabus) {
+error_setg(errp, "Unable to find a single ISA bus");
+return false;
+}
+d->irq[i] = isa_bus_get_irq(ISA_BUS(isabus), 14 + i);
+} else {
+error_setg(errp, "output IDE IRQ %u not connected", i);
+return false;
+}
 }

 ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
@@ -201,6 +212,13 @@ static void piix3_ide_class_init(ObjectClass 
*klass, void *data)

 k->class_id = PCI_CLASS_STORAGE_IDE;
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 dc->hotpluggable = false;
+/*
+ * This function is part of a Super I/O chip and shouldn't be user
+ * creatable. However QEMU accepts impossible hardware setups such
+ * plugging a PIIX IDE function on a ICH ISA bridge.
+ * Keep this Frankenstein (ab)use working.
+ */
+dc->user_creatable = true;
 }

 static const TypeInfo piix3_ide_info = {
@@ -224,6 +242,8 @@ static void piix4_ide_class_init(ObjectClass *klass, 
void *data)

 k->class_id = PCI_CLASS_STORAGE_IDE;
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 dc->hotpluggable = false;
+/* Reason: Part of a Super I/O chip */
+dc->user_creatable = false;
 }
---

But the hardware really looks Frankenstein now:

(qemu) info qom-tree
/machine (pc-q35-8.0-machine)
  /peripheral-anon (container)
/device[0] (piix3-ide)
  /bmdma[0] (memory-region)
  /bmdma[1] (memory-region)
  /bus master container[0] (memory-region)
  /bus master[0] (memory-region)
  /ide.6 (IDE)
  /ide.7 (IDE)
  /ide[0] (memory-region)
  /ide[1] (memory-region)
  /ide[2] (memory-region)
  /ide[3] (memory-region)
  /piix-bmdma-container[0] (memory-region)
  /piix-bmdma[0] 

[PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds.

[Andrew Melnychenko]

eBPF RSS program and maps may now be passed during initialization.
Initially was implemented for libvirt to launch qemu without permissions,
and initialized eBPF program through the helper.

Signed-off-by: Andrew Melnychenko 
---
 hw/net/virtio-net.c| 77 --
 include/hw/virtio/virtio-net.h |  1 +
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3ae909041a..0ab2cf33f9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -42,6 +42,7 @@
 #include "sysemu/sysemu.h"
 #include "trace.h"
 #include "monitor/qdev.h"
+#include "monitor/monitor.h"
 #include "hw/pci/pci_device.h"
 #include "net_rx_pkt.h"
 #include "hw/virtio/vhost.h"
@@ -1290,14 +1291,81 @@ static void virtio_net_detach_epbf_rss(VirtIONet *n)
 virtio_net_attach_ebpf_to_backend(n->nic, -1);
 }
 
-static bool virtio_net_load_ebpf(VirtIONet *n)
+static int virtio_net_get_ebpf_rss_fds(char *str, char *fds[], int nfds)
+{
+char *ptr = str;
+char *cur = NULL;
+size_t len = strlen(str);
+int i = 0;
+
+for (; i < nfds && ptr < str + len;) {
+cur = strchr(ptr, ':');
+
+if (cur == NULL) {
+fds[i] = g_strdup(ptr);
+} else {
+fds[i] = g_strndup(ptr, cur - ptr);
+}
+
+i++;
+if (cur == NULL) {
+break;
+} else {
+ptr = cur + 1;
+}
+}
+
+return i;
+}
+
+static bool virtio_net_load_ebpf_fds(VirtIONet *n)
 {
-if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
-/* backend does't support steering ebpf */
+char *fds_strs[EBPF_RSS_MAX_FDS];
+int fds[EBPF_RSS_MAX_FDS];
+int nfds;
+int ret = false;
+Error *errp;
+int i = 0;
+
+if (n == NULL || !n->ebpf_rss_fds) {
 return false;
 }
 
-return ebpf_rss_load(>ebpf_rss);
+nfds = virtio_net_get_ebpf_rss_fds(n->ebpf_rss_fds,
+   fds_strs, EBPF_RSS_MAX_FDS);
+for (i = 0; i < nfds; i++) {
+fds[i] = monitor_fd_param(monitor_cur(), fds_strs[i], );
+}
+
+if (nfds == EBPF_RSS_MAX_FDS) {
+ret = ebpf_rss_load_fds(>ebpf_rss, fds[0], fds[1], fds[2], fds[3]);
+}
+
+if (!ret) {
+for (i = 0; i < nfds; i++) {
+close(fds[i]);
+}
+}
+
+for (i = 0; i < nfds; i++) {
+g_free(fds_strs[i]);
+}
+
+return ret;
+}
+
+static bool virtio_net_load_ebpf(VirtIONet *n)
+{
+bool ret = true;
+
+if (virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
+if (!(n->ebpf_rss_fds
+&& virtio_net_load_ebpf_fds(n))) {
+ret = ebpf_rss_load(>ebpf_rss);
+}
+}
+
+return ret;
 }
 
 static void virtio_net_unload_ebpf(VirtIONet *n)
@@ -3868,6 +3936,7 @@ static Property virtio_net_properties[] = {
 VIRTIO_NET_F_RSS, false),
 DEFINE_PROP_BIT64("hash", VirtIONet, host_features,
 VIRTIO_NET_F_HASH_REPORT, false),
+DEFINE_PROP_STRING("ebpf_rss_fds", VirtIONet, ebpf_rss_fds),
 DEFINE_PROP_BIT64("guest_rsc_ext", VirtIONet, host_features,
 VIRTIO_NET_F_RSC_EXT, false),
 DEFINE_PROP_UINT32("rsc_interval", VirtIONet, rsc_timeout,
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index ef234ffe7e..e10ce88f91 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -219,6 +219,7 @@ struct VirtIONet {
 VirtioNetRssData rss_data;
 struct NetRxPkt *rx_pkt;
 struct EBPFRSSContext ebpf_rss;
+char *ebpf_rss_fds;
 };
 
 size_t virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
-- 
2.39.1

[PATCH 1/5] ebpf: Added eBPF initialization by fds and map update.

[Andrew Melnychenko]

Changed eBPF map updates through mmaped array.
Mmaped arrays provide direct access to map data.
It should omit using bpf_map_update_elem() call,
which may require capabilities that are not present.
Also, eBPF RSS context may be initialized by file descriptors.
virtio-net may provide fds passed by libvirt.

Signed-off-by: Andrew Melnychenko 
---
 ebpf/ebpf_rss-stub.c |   6 +++
 ebpf/ebpf_rss.c  | 120 ++-
 ebpf/ebpf_rss.h  |  10 
 3 files changed, 113 insertions(+), 23 deletions(-)

diff --git a/ebpf/ebpf_rss-stub.c b/ebpf/ebpf_rss-stub.c
index e71e229190..8d7fae2ad9 100644
--- a/ebpf/ebpf_rss-stub.c
+++ b/ebpf/ebpf_rss-stub.c
@@ -28,6 +28,12 @@ bool ebpf_rss_load(struct EBPFRSSContext *ctx)
 return false;
 }
 
+bool ebpf_rss_load_fds(struct EBPFRSSContext *ctx, int program_fd,
+   int config_fd, int toeplitz_fd, int table_fd)
+{
+return false;
+}
+
 bool ebpf_rss_set_all(struct EBPFRSSContext *ctx, struct EBPFRSSConfig *config,
   uint16_t *indirections_table, uint8_t *toeplitz_key)
 {
diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
index cee658c158..08015fecb1 100644
--- a/ebpf/ebpf_rss.c
+++ b/ebpf/ebpf_rss.c
@@ -27,19 +27,68 @@ void ebpf_rss_init(struct EBPFRSSContext *ctx)
 {
 if (ctx != NULL) {
 ctx->obj = NULL;
+ctx->program_fd = -1;
 }
 }
 
 bool ebpf_rss_is_loaded(struct EBPFRSSContext *ctx)
 {
-return ctx != NULL && ctx->obj != NULL;
+return ctx != NULL && (ctx->obj != NULL || ctx->program_fd != -1);
+}
+
+static bool ebpf_rss_mmap(struct EBPFRSSContext *ctx)
+{
+if (!ebpf_rss_is_loaded(ctx)) {
+return false;
+}
+
+ctx->mmap_configuration = mmap(NULL, qemu_real_host_page_size(),
+   PROT_READ | PROT_WRITE, MAP_SHARED,
+   ctx->map_configuration, 0);
+if (ctx->mmap_configuration == MAP_FAILED) {
+trace_ebpf_error("eBPF RSS", "can not mmap eBPF configuration array");
+return false;
+}
+ctx->mmap_toeplitz_key = mmap(NULL, qemu_real_host_page_size(),
+   PROT_READ | PROT_WRITE, MAP_SHARED,
+   ctx->map_toeplitz_key, 0);
+if (ctx->mmap_toeplitz_key == MAP_FAILED) {
+trace_ebpf_error("eBPF RSS", "can not mmap eBPF toeplitz key");
+goto toeplitz_fail;
+}
+ctx->mmap_indirections_table = mmap(NULL, qemu_real_host_page_size(),
+   PROT_READ | PROT_WRITE, MAP_SHARED,
+   ctx->map_indirections_table, 0);
+if (ctx->mmap_indirections_table == MAP_FAILED) {
+trace_ebpf_error("eBPF RSS", "can not mmap eBPF indirection table");
+goto indirection_fail;
+}
+
+return true;
+
+indirection_fail:
+munmap(ctx->mmap_toeplitz_key, qemu_real_host_page_size());
+toeplitz_fail:
+munmap(ctx->mmap_configuration, qemu_real_host_page_size());
+return false;
+}
+
+static void ebpf_rss_munmap(struct EBPFRSSContext *ctx)
+{
+if (!ebpf_rss_is_loaded(ctx)) {
+return;
+}
+
+munmap(ctx->mmap_indirections_table, qemu_real_host_page_size());
+munmap(ctx->mmap_toeplitz_key, qemu_real_host_page_size());
+munmap(ctx->mmap_configuration, qemu_real_host_page_size());
 }
 
 bool ebpf_rss_load(struct EBPFRSSContext *ctx)
 {
 struct rss_bpf *rss_bpf_ctx;
 
-if (ctx == NULL) {
+if (ctx == NULL || ebpf_rss_is_loaded(ctx)) {
 return false;
 }
 
@@ -66,26 +115,51 @@ bool ebpf_rss_load(struct EBPFRSSContext *ctx)
 ctx->map_toeplitz_key = bpf_map__fd(
 rss_bpf_ctx->maps.tap_rss_map_toeplitz_key);
 
+if (!ebpf_rss_mmap(ctx)) {
+goto error;
+}
+
 return true;
 error:
 rss_bpf__destroy(rss_bpf_ctx);
 ctx->obj = NULL;
+ctx->program_fd = -1;
 
 return false;
 }
 
-static bool ebpf_rss_set_config(struct EBPFRSSContext *ctx,
-struct EBPFRSSConfig *config)
+bool ebpf_rss_load_fds(struct EBPFRSSContext *ctx, int program_fd,
+   int config_fd, int toeplitz_fd, int table_fd)
 {
-uint32_t map_key = 0;
+if (ctx == NULL || ebpf_rss_is_loaded(ctx)) {
+return false;
+}
 
-if (!ebpf_rss_is_loaded(ctx)) {
+if (program_fd < 0 || config_fd < 0 || toeplitz_fd < 0 || table_fd < 0) {
 return false;
 }
-if (bpf_map_update_elem(ctx->map_configuration,
-_key, config, 0) < 0) {
+
+ctx->program_fd = program_fd;
+ctx->map_configuration = config_fd;
+ctx->map_toeplitz_key = toeplitz_fd;
+ctx->map_indirections_table = table_fd;
+
+if (!ebpf_rss_mmap(ctx)) {
+ctx->program_fd = -1;
 return false;
 }
+
+return true;
+}
+
+static bool ebpf_rss_set_config(struct EBPFRSSContext *ctx,
+struct EBPFRSSConfig *config)
+{
+if (!ebpf_rss_is_loaded(ctx)) {
+  

[PATCH 5/5] qmp: Added find-ebpf-rss-helper command.

[Andrew Melnychenko]

New qmp command to query ebpf helper.
It's crucial that QEMU and helper are in sync.
Technically helper should pass eBPF fds that QEMU may accept.
And different QEMU's builds may have different eBPF programs.
QEMU returns helper that should "fit" to virtio-net.
QEMU would check the stamp of the helper to make sure
that eBPF program is valid.

Signed-off-by: Andrew Melnychenko 
---
 monitor/qmp-cmds.c | 28 
 qapi/misc.json | 42 ++
 2 files changed, 70 insertions(+)

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 859012aef4..2f91c34bbb 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -31,6 +31,7 @@
 #include "hw/mem/memory-device.h"
 #include "hw/intc/intc.h"
 #include "hw/rdma/rdma.h"
+#include "qemu-ebpf-rss-helper-stamp-utils.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -202,3 +203,30 @@ static void __attribute__((__constructor__)) 
monitor_init_qmp_commands(void)
  qmp_marshal_qmp_capabilities,
  QCO_ALLOW_PRECONFIG, 0);
 }
+
+HelperPath *qmp_find_ebpf_rss_helper(bool has_path,
+ strList *path, Error **errp)
+{
+HelperPath *ret = NULL;
+char *helperbin = NULL;
+
+/* Look for helper in the suggested pathes */
+if (has_path) {
+strList *str_list = NULL;
+for (str_list = path;
+ str_list && !helperbin;
+ str_list = str_list->next) {
+helperbin = qemu_check_suggested_ebpf_rss_helper(str_list->value);
+}
+}
+
+if (helperbin == NULL) {
+helperbin = qemu_find_default_ebpf_rss_helper();
+}
+
+if (helperbin) {
+ret = g_new0(HelperPath, 1);
+ret->path = helperbin;
+}
+return ret;
+}
diff --git a/qapi/misc.json b/qapi/misc.json
index 27ef5a2b20..1dfb3c132e 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -584,3 +584,45 @@
 { 'event': 'VFU_CLIENT_HANGUP',
   'data': { 'vfu-id': 'str', 'vfu-qom-path': 'str',
 'dev-id': 'str', 'dev-qom-path': 'str' } }
+
+##
+# @HelperPath:
+#
+# Name of the helper and binary location.
+##
+{ 'struct': 'HelperPath',
+  'data': {'path': 'str'} }
+
+##
+# @find-ebpf-rss-helper:
+#
+# Query helper paths to find qemu-ebpf-rss-helper.
+# The qemu would check "the stamp" and
+# returns the proper helper.
+# It's possible to provide paths where to look for a helper.
+# If the path is provided to a file - qemu would check the file for the stamp.
+# If the path is provided to a directory - qemu would look for
+# a file "qemu-ebpf-rss-helper" and check its stamp.
+#
+# Returns: path to the helper with a valid stamp.
+#
+# Note: Provided path arguments have the highest priority where to look
+#   for a helper. Then, default "helper directory" and then
+#   near the qemu binary.
+#
+# Since: 7.2
+#
+# Example:
+#
+# -> { "execute": "find-ebpf-rss-helper", "arguments": { "path": 
"/opt/qemu_helpers/" } }
+# <- { "return": [
+#{
+#  "path": "/usr/local/libexec/qemu-ebpf-rss-helper"
+#}
+#  ]
+#}
+#
+##
+{ 'command': 'find-ebpf-rss-helper',
+  'data': {'*path': ['str']},
+  'returns': 'HelperPath' }
-- 
2.39.1

[PATCH 0/5] eBPF RSS Helper support.

[Andrew Melnychenko]

This series of patches provides the ability to initialize eBPF RSS steering
with the helper.
Now, virtio-net devices can accept eBPF programs and maps through properties
as external file descriptors. Access to the eBPF map is direct through mmap()
call, so it should not require additional capabilities to bpf* calls.
eBPF file descriptors can be passed to QEMU from parent process or by unix
socket with sendfd() qmp command.
The helper is provided that would load eBPF RSS program/maps and pass fd to
the called process(in future - Libvirtd) through unix socket.
Because of structures stored in the maps, it's crucial that the helper provides 
a proper eBPF program. That's why the stamp was added to the helper and
QEMU may check the binary. Also, additional qmp command was added for checking
the stamp.
Overall, the basic scenario of using the helper looks like this:
 * Libvirt checks for ebpf_fds property.
 * Libvirt ask QEMU for the proper helper(where is located and proper stamp)
 * Libvirt calls the helper with BPF capabilities and retrieves fds.
 * Libvirt launches the QEMU with eBPF fds passed.

Changes since RFC:
 * refactored/rebased code.
 * changed qmp command.
 * refactored helper.
 
Andrew Melnychenko (5):
  ebpf: Added eBPF initialization by fds and map update.
  virtio-net: Added property to load eBPF RSS with fds.
  qmp: Added the helper stamp check.
  ebpf_rss_helper: Added helper for eBPF RSS.
  qmp: Added find-ebpf-rss-helper command.

 ebpf/ebpf_rss-stub.c   |   6 +
 ebpf/ebpf_rss.c| 120 ++--
 ebpf/ebpf_rss.h|  10 +
 ebpf/qemu-ebpf-rss-helper.c| 132 +
 hw/net/virtio-net.c|  77 -
 include/hw/virtio/virtio-net.h |   1 +
 meson.build|  47 ++-
 monitor/meson.build|   1 +
 monitor/qemu-ebpf-rss-helper-stamp-utils.c | 322 +
 monitor/qemu-ebpf-rss-helper-stamp-utils.h |  39 +++
 monitor/qmp-cmds.c |  28 ++
 qapi/misc.json |  42 +++
 12 files changed, 785 insertions(+), 40 deletions(-)
 create mode 100644 ebpf/qemu-ebpf-rss-helper.c
 create mode 100644 monitor/qemu-ebpf-rss-helper-stamp-utils.c
 create mode 100644 monitor/qemu-ebpf-rss-helper-stamp-utils.h

-- 
2.39.1

[PATCH 3/5] qmp: Added the helper stamp check.

[Andrew Melnychenko]

Added a function to check the stamp in the helper.
eBPF helper should have a special symbol that generates during the build.
QEMU checks the helper and determines that it fits, so the helper
will produce proper output.

Signed-off-by: Andrew Melnychenko 
---
 meson.build|  10 +
 monitor/meson.build|   1 +
 monitor/qemu-ebpf-rss-helper-stamp-utils.c | 322 +
 monitor/qemu-ebpf-rss-helper-stamp-utils.h |  39 +++
 4 files changed, 372 insertions(+)
 create mode 100644 monitor/qemu-ebpf-rss-helper-stamp-utils.c
 create mode 100644 monitor/qemu-ebpf-rss-helper-stamp-utils.h

diff --git a/meson.build b/meson.build
index a76c855312..b409912aed 100644
--- a/meson.build
+++ b/meson.build
@@ -2868,6 +2868,16 @@ foreach d : hx_headers
 endforeach
 genh += hxdep
 
+ebpf_rss_helper_stamp = custom_target(
+'qemu-ebpf-rss-helper-stamp.h',
+output : 'qemu-ebpf-rss-helper-stamp.h',
+input : 'ebpf/rss.bpf.skeleton.h',
+command : [python, '-c', 'import hashlib; print(\'#define 
QEMU_EBPF_RSS_HELPER_STAMP 
qemuEbpfRssHelperStamp_{}\'.format(hashlib.sha1(open(\'@INPUT@\', 
\'rb\').read()).hexdigest()))'],
+capture: true,
+)
+
+genh += ebpf_rss_helper_stamp
+
 ###
 # Collect sources #
 ###
diff --git a/monitor/meson.build b/monitor/meson.build
index ccb4d1a8e6..36de73414b 100644
--- a/monitor/meson.build
+++ b/monitor/meson.build
@@ -6,6 +6,7 @@ softmmu_ss.add(files(
   'hmp.c',
 ))
 softmmu_ss.add([spice_headers, files('qmp-cmds.c')])
+softmmu_ss.add(files('qemu-ebpf-rss-helper-stamp-utils.c'))
 
 specific_ss.add(when: 'CONFIG_SOFTMMU',
if_true: [files( 'hmp-cmds-target.c', 'hmp-target.c'), spice])
diff --git a/monitor/qemu-ebpf-rss-helper-stamp-utils.c 
b/monitor/qemu-ebpf-rss-helper-stamp-utils.c
new file mode 100644
index 00..23efc36ef0
--- /dev/null
+++ b/monitor/qemu-ebpf-rss-helper-stamp-utils.c
@@ -0,0 +1,322 @@
+/*
+ * QEMU helper stamp check utils.
+ *
+ * Developed by Daynix Computing LTD (http://www.daynix.com)
+ *
+ * Authors:
+ *  Andrew Melnychenko 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Description: This file mostly implements helper stamp checking.
+ *  The stamp is implemented in a similar way as in qemu modules.
+ *  The helper should contain a specific symbol.
+ *  Not in a similar way is symbol checking - here we parse
+ *  the ELF file. For now, only eBPF helper contains
+ *  the stamp, and the stamp is generated from
+ *  sha1 ebpf/rss.bpf.skeleton.h (see meson.build).
+ */
+
+#include "qemu/osdep.h"
+#include "elf.h"
+#include "qemu-ebpf-rss-helper-stamp-utils.h"
+
+#include 
+
+#ifdef CONFIG_LINUX
+
+static void *file_allocate_and_read(int fd, off_t off, size_t size)
+{
+void *data;
+int err;
+
+if (fd < 0) {
+return NULL;
+}
+
+err = lseek(fd, off, SEEK_SET);
+if (err < 0) {
+return NULL;
+}
+
+data = g_new0(char, size);
+if (data == NULL) {
+return NULL;
+}
+
+err = read(fd, data, size);
+if (err < 0) {
+g_free(data);
+return NULL;
+}
+
+return data;
+}
+
+static Elf64_Shdr *elf64_get_section_table(int fd, Elf64_Ehdr *elf_header)
+{
+if (elf_header == NULL) {
+return NULL;
+}
+return (Elf64_Shdr *)file_allocate_and_read(fd, elf_header->e_shoff,
+ elf_header->e_shnum * elf_header->e_shentsize);
+}
+
+static Elf32_Shdr *elf32_get_section_table(int fd, Elf32_Ehdr *elf_header)
+{
+if (elf_header == NULL) {
+return NULL;
+}
+return (Elf32_Shdr *)file_allocate_and_read(fd, elf_header->e_shoff,
+ elf_header->e_shnum * elf_header->e_shentsize);
+}
+
+static void *elf64_get_section_data(int fd, const Elf64_Shdr* section_header)
+{
+if (fd < 0 || section_header == NULL) {
+return NULL;
+}
+return file_allocate_and_read(fd, section_header->sh_offset,
+  section_header->sh_size);
+}
+
+static void *elf32_get_section_data(int fd, const Elf32_Shdr* section_header)
+{
+if (fd < 0 || section_header == NULL) {
+return NULL;
+}
+return file_allocate_and_read(fd, section_header->sh_offset,
+  section_header->sh_size);
+}
+
+static bool elf64_check_symbol_in_symbol_table(int fd,
+   Elf64_Shdr *section_table,
+   Elf64_Shdr *symbol_section,
+   const char *symbol)
+{
+Elf64_Sym *symbol_table;
+char *string_table;
+uint32_t i;
+bool ret = false;
+
+symbol_table = (Elf64_Sym *) elf64_get_section_data(fd, symbol_section);
+if (symbol_table == NULL) {
+return 

[PATCH 4/5] ebpf_rss_helper: Added helper for eBPF RSS.

[Andrew Melnychenko]

Helper program. Loads eBPF RSS program and maps and passes them through unix 
socket.
Libvirt may launch this helper and pass eBPF fds to qemu virtio-net.
Also, libbpf dependency for now is exclusively for Linux.
Libbpf is used for eBPF RSS steering, which is supported only by Linux TAP.
There is no reason yet to build eBPF loader and helper for non-Linux systems,
even if libbpf is present.

Signed-off-by: Andrew Melnychenko 
---
 ebpf/qemu-ebpf-rss-helper.c | 132 
 meson.build |  37 ++
 2 files changed, 156 insertions(+), 13 deletions(-)
 create mode 100644 ebpf/qemu-ebpf-rss-helper.c

diff --git a/ebpf/qemu-ebpf-rss-helper.c b/ebpf/qemu-ebpf-rss-helper.c
new file mode 100644
index 00..348d26bcdd
--- /dev/null
+++ b/ebpf/qemu-ebpf-rss-helper.c
@@ -0,0 +1,132 @@
+/*
+ * eBPF RSS Helper
+ *
+ * Developed by Daynix Computing LTD (http://www.daynix.com)
+ *
+ * Authors:
+ *  Andrew Melnychenko 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Description: This is helper program for libvirtd.
+ *  It loads eBPF RSS program and passes fds through unix socket.
+ *  Built by meson, target - 'qemu-ebpf-rss-helper'.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ebpf_rss.h"
+
+#include "qemu-ebpf-rss-helper-stamp.h"
+
+void QEMU_EBPF_RSS_HELPER_STAMP(void) {}
+
+static int send_fds(int socket, int *fds, int n)
+{
+struct msghdr msg = {};
+struct cmsghdr *cmsg = NULL;
+char buf[CMSG_SPACE(n * sizeof(int))];
+char dummy_buffer = 0;
+struct iovec io = { .iov_base = _buffer,
+.iov_len = sizeof(dummy_buffer) };
+
+memset(buf, 0, sizeof(buf));
+
+msg.msg_iov = 
+msg.msg_iovlen = 1;
+msg.msg_control = buf;
+msg.msg_controllen = sizeof(buf);
+
+cmsg = CMSG_FIRSTHDR();
+cmsg->cmsg_level = SOL_SOCKET;
+cmsg->cmsg_type = SCM_RIGHTS;
+cmsg->cmsg_len = CMSG_LEN(n * sizeof(int));
+
+memcpy(CMSG_DATA(cmsg), fds, n * sizeof(int));
+
+return sendmsg(socket, , 0);
+}
+
+static void print_help_and_exit(const char *prog, int exitcode)
+{
+fprintf(stderr, "%s - load eBPF RSS program for qemu and pass eBPF fds"
+" through unix socket.\n", prog);
+fprintf(stderr, "\t--fd , -f  - unix socket file descriptor"
+" used to pass eBPF fds.\n");
+fprintf(stderr, "\t--help, -h - this help.\n");
+exit(exitcode);
+}
+
+int main(int argc, char **argv)
+{
+char *fd_string = NULL;
+int unix_fd = 0;
+struct EBPFRSSContext ctx = {};
+int fds[EBPF_RSS_MAX_FDS] = {};
+int ret = -1;
+
+for (;;) {
+int c;
+static struct option long_options[] = {
+{"help",  no_argument, 0, 'h'},
+{"fd",  required_argument, 0, 'f'},
+{0, 0, 0, 0}
+};
+c = getopt_long(argc, argv, "hf:",
+long_options, NULL);
+
+if (c == -1) {
+break;
+}
+
+switch (c) {
+case 'f':
+fd_string = optarg;
+break;
+case 'h':
+default:
+print_help_and_exit(argv[0],
+c == 'h' ? EXIT_SUCCESS : EXIT_FAILURE);
+}
+}
+
+if (!fd_string) {
+fprintf(stderr, "Unix file descriptor not present.\n");
+print_help_and_exit(argv[0], EXIT_FAILURE);
+}
+
+unix_fd = atoi(fd_string);
+
+if (!unix_fd) {
+fprintf(stderr, "Unix file descriptor is invalid.\n");
+return EXIT_FAILURE;
+}
+
+ebpf_rss_init();
+if (!ebpf_rss_load()) {
+fprintf(stderr, "Can't load ebpf.\n");
+return EXIT_FAILURE;
+}
+fds[0] = ctx.program_fd;
+fds[1] = ctx.map_configuration;
+fds[2] = ctx.map_toeplitz_key;
+fds[3] = ctx.map_indirections_table;
+
+ret = send_fds(unix_fd, fds, EBPF_RSS_MAX_FDS);
+if (ret < 0) {
+fprintf(stderr, "Issue while sending fds: %s.\n", strerror(errno));
+}
+
+ebpf_rss_unload();
+
+return ret < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+}
+
diff --git a/meson.build b/meson.build
index b409912aed..6e6e2f3e40 100644
--- a/meson.build
+++ b/meson.build
@@ -1632,19 +1632,22 @@ elif get_option('vduse_blk_export').disabled()
 endif
 
 # libbpf
-libbpf = dependency('libbpf', required: get_option('bpf'), method: 
'pkg-config')
-if libbpf.found() and not cc.links('''
-   #include 
-   int main(void)
-   {
- bpf_object__destroy_skeleton(NULL);
- return 0;
-   }''', dependencies: libbpf)
-  libbpf = not_found
-  if get_option('bpf').enabled()
-error('libbpf skeleton test failed')
-  else
-warning('libbpf skeleton test failed, disabling')
+libbpf = not_found
+if targetos == 'linux'
+  libbpf = dependency('libbpf', required: get_option('bpf'), method: 
'pkg-config')
+  if libbpf.found() and not 

Re: [PATCH v4 8/8] hw/mem/cxl_type3: Add CXL RAS Error Injection Support.

[Jonathan Cameron via]

> >   static void ct3d_reg_write(void *opaque, hwaddr offset, uint64_t value,
> >  unsigned size)
> >   {
> > @@ -341,6 +402,83 @@ static void ct3d_reg_write(void *opaque, hwaddr 
> > offset, uint64_t value,
> >   should_commit = FIELD_EX32(value, CXL_HDM_DECODER0_CTRL, COMMIT);
> >   which_hdm = 0;
> >   break;
> > +case A_CXL_RAS_UNC_ERR_STATUS:
> > +{
> > +uint32_t capctrl = ldl_le_p(cache_mem + R_CXL_RAS_ERR_CAP_CTRL);
> > +uint32_t fe = FIELD_EX32(capctrl, CXL_RAS_ERR_CAP_CTRL, 
> > FIRST_ERROR_POINTER);
> > +CXLError *cxl_err;
> > +uint32_t unc_err;
> > +
> > +/*
> > + * If single bit written that corresponds to the first error
> > + * pointer being cleared, update the status and header log.
> > + */
> > +if (!QTAILQ_EMPTY(>error_list)) {
> > +if ((1 << fe) ^ value) {
> > +CXLError *cxl_next;
> > +/*
> > + * Software is using wrong flow for multiple header 
> > recording
> > + * Following behaviour in PCIe r6.0 and assuming multiple
> > + * header support. Imdef choice to clear all matching 
> > records  
> 
> What does "Imdef" mean?

Good spot. Should have been Impdef, but there is no reason not to spell it out
as "Implementation defined".
What I'm trying to indicate here is that the PCIe r6.0 base specification lets
hardware do one of several different things.  I picked one of those options.

In PCIe it's a little less critical than in CXL as there is an explicit opt in
so you can expect software to do the right thing.  Unfortunately not so for
CXL where we have to assume the capability being there means the hardware will
do it.  I guess there was no need for backwards compatibility for CXL.

Jonathan

> 
> DJ

Re: [PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Corey Minyard]

On Sun, Feb 19, 2023 at 02:45:44PM +0100, Philippe Mathieu-Daudé wrote:
> On 18/2/23 21:25, Corey Minyard wrote:
> > On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote:
> > > ich9_smb_init() is a legacy init function, so modernize the code.
> > > 
> > > Note that the smb_io_base parameter was unused.
> > 
> > Acked-by: Corey Minyard 
> > 
> > > 
> > > Signed-off-by: Bernhard Beschow 
> > > ---
> > >   include/hw/i386/ich9.h |  1 -
> > >   hw/i2c/smbus_ich9.c| 13 +++--
> > >   hw/i386/pc_q35.c   | 11 ---
> > >   3 files changed, 11 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> > > index 05464f6965..52ea116f44 100644
> > > --- a/include/hw/i386/ich9.h
> > > +++ b/include/hw/i386/ich9.h
> > > @@ -9,7 +9,6 @@
> > >   #include "qom/object.h"
> > >   void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
> > > -I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
> > >   void ich9_generate_smi(void);
> > > diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
> > > index d29c0f6ffa..f0dd3cb147 100644
> > > --- a/hw/i2c/smbus_ich9.c
> > > +++ b/hw/i2c/smbus_ich9.c
> > > @@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error 
> > > **errp)
> > >   pm_smbus_init(>qdev, >smb, false);
> > >   pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, 
> > > PCI_BASE_ADDRESS_SPACE_IO,
> > >>smb.io);
> > > +
> > > +s->smb.set_irq = ich9_smb_set_irq;
> > > +s->smb.opaque = s;
> 
> Corey, shouldn't we expand pm_smbus_init() to take set_irq / opaque
> arguments?

That would be nice, but the other two user of this function,
hw/isa/vt82c686.c and hw/acpi/piix4.c, don't use these fields.
I doubt we are getting any new users.  I'm fine either way, but the
value is not large.

-corey

> 
> > >   }
> 

Re: [PATCH 05/12] hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it

[Philippe Mathieu-Daudé]

On 18/2/23 21:25, Corey Minyard wrote:

On Mon, Feb 13, 2023 at 06:30:26PM +0100, Bernhard Beschow wrote:

ich9_smb_init() is a legacy init function, so modernize the code.

Note that the smb_io_base parameter was unused.


Acked-by: Corey Minyard 



Signed-off-by: Bernhard Beschow 
---
  include/hw/i386/ich9.h |  1 -
  hw/i2c/smbus_ich9.c| 13 +++--
  hw/i386/pc_q35.c   | 11 ---
  3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 05464f6965..52ea116f44 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -9,7 +9,6 @@
  #include "qom/object.h"
  
  void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);

-I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
  
  void ich9_generate_smi(void);
  
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c

index d29c0f6ffa..f0dd3cb147 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -105,6 +105,9 @@ static void ich9_smbus_realize(PCIDevice *d, Error **errp)
  pm_smbus_init(>qdev, >smb, false);
  pci_register_bar(d, ICH9_SMB_SMB_BASE_BAR, PCI_BASE_ADDRESS_SPACE_IO,
   >smb.io);
+
+s->smb.set_irq = ich9_smb_set_irq;
+s->smb.opaque = s;


Corey, shouldn't we expand pm_smbus_init() to take set_irq / opaque
arguments?


  }

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

[Stefan Weil via]

Am 19.02.23 um 12:27 schrieb Reinoud Zandijk:


On Fri, Feb 17, 2023 at 12:05:46PM +0100, Stefan Weil wrote:

So there still seems to be a certain small need for QEMU installers for
32-bit Windows: 170 users für 32 bit only, 339 users for both 32 and 64 bit,
5132 users for 64 bit only.

As you seem to have access to download stats could you check generic download
stats too i.e. not only for Windows installers?



Sorry, but I only have statistics for my own server 
https://qemu.weilnetz.de/ which provides Windows installers.


Stefan

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

[Reinoud Zandijk]

On Fri, Feb 17, 2023 at 12:05:46PM +0100, Stefan Weil wrote:
> On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
> 
> > Which 32-bit hosts are still useful, and why?
> 
> Citing my previous mail:
> 
>I now checked all downloads of the latests installers since 2022-12-30.
> 
>qemu-w32-setup-20221230.exe – 509 different IP addresses
>qemu-w64-setup-20221230.exe - 5471 different IP addresses
> 
>339 unique IP addresses are common for 32- and 64-bit, either
>crawlers or people who simply got both variants. So there remain 170
>IP addresses which only downloaded the 32-bit variant in the last week.
> 
>I see 437 different strings for the browser type, but surprisingly
>none of them looks like a crawler.
> 
> So there still seems to be a certain small need for QEMU installers for
> 32-bit Windows: 170 users für 32 bit only, 339 users for both 32 and 64 bit,
> 5132 users for 64 bit only.

As you seem to have access to download stats could you check generic download
stats too i.e. not only for Windows installers?

Also, how do you account for all 32 bit host downloads from say mirrors or the
source code downloads that are compiled and redistributed by 3rd parties for
32 bit hosts?

Although I am not that happy with dropping support on any platform I don't
think such a decision should be taken that lightly.

Maybe a change in the test setup would be handier? I.e. only test 32 bit hosts
say once a 2 days and the 64 bit hosts each day?

Reinoud

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

[Reinoud Zandijk]

On Fri, Feb 17, 2023 at 08:22:43AM -1000, Richard Henderson wrote:
> On 2/17/23 06:06, Reinoud Zandijk wrote:
> > On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:
> > > I feel the discussion petered out without a conclusion.
> > > 
> > > I don't think letting the status quo win by inertia is a good outcome
> > > here.
> > > 
> > > Which 32-bit hosts are still useful, and why?
> > 
> > NetBSD runs on a bunch of 32 bit-only hosts (sparc32, ppc32, armv7, vax,
> > mips32 etc.) that all run Qemu fine. They are all actively maintained and
> > released as part of the main releases.
> 
> Are you sure about that?  TCG doesn't support sparc32 or vax.
> I suppose you could be using TCI, but I can't even imagine how
> slow that would be on vax.

I've asked around amongst active developers and they steted that they had used
it in the past but not ianymore recently. I doubt normal users would use them
often though since most hardware is 64 bit capable anyways.

Reinoud

Re: [PATCH] target/ppc/translate: Add dummy implementation for dcblc instruction

[Bernhard Beschow]

Am 30. Januar 2023 22:23:59 UTC schrieb Richard Henderson 
:
>On 1/30/23 08:49, Bernhard Beschow wrote:
>> The dcblc instruction is used by u-boot in mpc85xx/start.S. Without it,
>> an illegal istruction exception is generated very early in the boot
>> process where the processor is not yet able to handle exceptions. See:
>> 
>> https://github.com/u-boot/u-boot/blob/v2023.01/arch/powerpc/cpu/mpc85xx/start.S#L1840
>> 
>> Signed-off-by: Bernhard Beschow 
>> ---
>>   target/ppc/translate.c | 9 +
>>   1 file changed, 9 insertions(+)
>> 
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index edb3daa9b5..8c32e697d9 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -5261,6 +5261,14 @@ static void gen_dcbtls(DisasContext *ctx)
>>   tcg_temp_free(t0);
>>   }
>>   +/* dcblc */
>> +static void gen_dcblc(DisasContext *ctx)
>> +{
>> +/*
>> + * interpreted as no-op
>> + */
>
>Missing
>
>If MSR[UCLE] (user-mode cache lock enable) is set, dcblc[e] may be performed 
>while in
>user mode (MSR[PR] = 1). If MSR[UCLE] is clear, an attempt to perform this 
>instructions
>in user mode causes a DSI. ESR[DLK] is set for this DSI.
>
>but that's also true for the current implementation of dcbtls.  So,
>
>Acked-by: Richard Henderson 

Ping

Does that mean it's ready to be queued or do we need a Reviewed-by as well?
>
>
>r~

Re: [PATCH 4/4] ui/gtk: enable backend to send multi-touch events

[Marc-André Lureau]

Hi Sergio

On Sat, Feb 18, 2023 at 8:23 PM Sergio Lopez  wrote:
>
> GTK3 provides the infrastructure to receive and process multi-touch
> events through the "touch-event" signal and the GdkEventTouch type.
> Make use of it to transpose events from the host to the guest.
>
> This allows users of machines with hardware capable of receiving
> multi-touch events to run guests that can also receive those events
> and interpret them as gestures, when appropriate.
>
> An example of this in action can be seen here:
>
>  https://fosstodon.org/@slp/109545849296546767
>
> Signed-off-by: Sergio Lopez 

> ---
>  ui/gtk.c | 84 
>  1 file changed, 84 insertions(+)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index fd82e9b1ca..bf1e7f086d 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -130,6 +130,13 @@ typedef struct VCChardev VCChardev;
>  DECLARE_INSTANCE_CHECKER(VCChardev, VC_CHARDEV,
>   TYPE_CHARDEV_VC)
>
> +struct touch_slot {
> +int x;
> +int y;
> +int tracking_id;
> +};
> +static struct touch_slot touch_slots[INPUT_EVENT_SLOTS_MAX];
> +
>  bool gtk_use_gl_area;
>
>  static void gd_grab_pointer(VirtualConsole *vc, const char *reason);
> @@ -1058,6 +1065,74 @@ static gboolean gd_scroll_event(GtkWidget *widget, 
> GdkEventScroll *scroll,
>  }
>
>
> +static gboolean gd_touch_event(GtkWidget *widget, GdkEventTouch *touch,
> +   void *opaque)
> +{
> +VirtualConsole *vc = opaque;
> +struct touch_slot *slot;
> +uint64_t num_slot = (uint64_t) touch->sequence;

Perhaps use GPOINTER_TO_UINT?

> +int update;
> +int type = -1;
> +int i;
> +
> +if (num_slot >= INPUT_EVENT_SLOTS_MAX) {
> +return FALSE;
> +}

Hmm, a pointer < INPUT_EVENT_SLOTS_MAX ?

This seems to work because the wayland GDK backend uses presumably
evdev slot id + 1.. We may want to have some slot id mapping, or at
least report some warning for discarded events.

> +
> +slot = _slots[num_slot];
> +slot->x = touch->x;
> +slot->y = touch->y;
> +
> +switch (touch->type) {
> +case GDK_TOUCH_BEGIN:
> +type = INPUT_MULTITOUCH_TYPE_BEGIN;
> +slot->tracking_id = num_slot;
> +break;
> +case GDK_TOUCH_UPDATE:
> +type = INPUT_MULTITOUCH_TYPE_UPDATE;
> +break;
> +case GDK_TOUCH_END:
> +case GDK_TOUCH_CANCEL:
> +type = INPUT_MULTITOUCH_TYPE_END;
> +break;
> +default:
> +fprintf(stderr, "%s: unexpected touch event\n", __func__);
> +}
> +
> +for (i = 0; i < INPUT_EVENT_SLOTS_MAX; ++i) {
> +if (i == num_slot) {
> +update = type;
> +} else {
> +update = INPUT_MULTITOUCH_TYPE_UPDATE;
> +}
> +
> +slot = _slots[i];
> +
> +if (slot->tracking_id == -1) {
> +continue;
> +}
> +
> +if (update == INPUT_MULTITOUCH_TYPE_END) {
> +slot->tracking_id = -1;
> +qemu_input_queue_mtt(vc->gfx.dcl.con, update, i, 
> slot->tracking_id);
> +} else {
> +qemu_input_queue_mtt(vc->gfx.dcl.con, update, i, 
> slot->tracking_id);
> +qemu_input_queue_btn(vc->gfx.dcl.con, INPUT_BUTTON_TOUCH, true);
> +qemu_input_queue_mtt_abs(vc->gfx.dcl.con,
> + INPUT_AXIS_X, (int) slot->x,
> + 0, surface_width(vc->gfx.ds),
> + i, slot->tracking_id);
> +qemu_input_queue_mtt_abs(vc->gfx.dcl.con,
> + INPUT_AXIS_Y, (int) slot->y,
> + 0, surface_height(vc->gfx.ds),
> + i, slot->tracking_id);
> +}
> +qemu_input_event_sync();

Shouldn't you sync at the end of the loop? (otherwise you get several
SYN_REPORT, no?)

> +}
> +
> +return TRUE;
> +}
> +
>  static const guint16 *gd_get_keymap(size_t *maplen)
>  {
>  GdkDisplay *dpy = gdk_display_get_default();
> @@ -1977,6 +2052,8 @@ static void gd_connect_vc_gfx_signals(VirtualConsole 
> *vc)
>   G_CALLBACK(gd_key_event), vc);
>  g_signal_connect(vc->gfx.drawing_area, "key-release-event",
>   G_CALLBACK(gd_key_event), vc);
> +g_signal_connect(vc->gfx.drawing_area, "touch-event",
> + G_CALLBACK(gd_touch_event), vc);
>
>  g_signal_connect(vc->gfx.drawing_area, "enter-notify-event",
>   G_CALLBACK(gd_enter_event), vc);
> @@ -2086,6 +2163,7 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, 
> VirtualConsole *vc,
>GSList *group, GtkWidget *view_menu)
>  {
>  bool zoom_to_fit = false;
> +int i;
>
>  vc->label = qemu_console_get_label(con);
>  vc->s = s;
> @@ -2133,6 +2211,7 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, 
> VirtualConsole *vc,
> 

Re: [PATCH 1/4] virtio-input: generalize virtio_input_key_config()

[Marc-André Lureau]

On Sat, Feb 18, 2023 at 8:23 PM Sergio Lopez  wrote:
>
> As there are other bitmap-based config properties that need to be dealt in a
> similar fashion as VIRTIO_INPUT_CFG_EV_BITS, generalize the function to
> receive select and subsel as arguments, and rename it to
> virtio_input_extend_config()
>
> Signed-off-by: Sergio Lopez 

Reviewed-by: Marc-André Lureau 



> ---
>  hw/input/virtio-input-hid.c | 38 -
>  1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
> index a7a244a95d..d28dab69ba 100644
> --- a/hw/input/virtio-input-hid.c
> +++ b/hw/input/virtio-input-hid.c
> @@ -44,30 +44,31 @@ static const unsigned short axismap_abs[INPUT_AXIS__MAX] 
> = {
>
>  /* - */
>
> -static void virtio_input_key_config(VirtIOInput *vinput,
> -const unsigned short *keymap,
> -size_t mapsize)
> +static void virtio_input_extend_config(VirtIOInput *vinput,
> +   const unsigned short *map,
> +   size_t mapsize,
> +   uint8_t select, uint8_t subsel)
>  {
> -virtio_input_config keys;
> +virtio_input_config ext;
>  int i, bit, byte, bmax = 0;
>
> -memset(, 0, sizeof(keys));
> +memset(, 0, sizeof(ext));
>  for (i = 0; i < mapsize; i++) {
> -bit = keymap[i];
> +bit = map[i];
>  if (!bit) {
>  continue;
>  }
>  byte = bit / 8;
>  bit  = bit % 8;
> -keys.u.bitmap[byte] |= (1 << bit);
> +ext.u.bitmap[byte] |= (1 << bit);
>  if (bmax < byte+1) {
>  bmax = byte+1;
>  }
>  }
> -keys.select = VIRTIO_INPUT_CFG_EV_BITS;
> -keys.subsel = EV_KEY;
> -keys.size   = bmax;
> -virtio_input_add_config(vinput, );
> +ext.select = select;
> +ext.subsel = subsel;
> +ext.size   = bmax;
> +virtio_input_add_config(vinput, );
>  }
>
>  static void virtio_input_handle_event(DeviceState *dev, QemuConsole *src,
> @@ -281,8 +282,9 @@ static void virtio_keyboard_init(Object *obj)
>
>  vhid->handler = _keyboard_handler;
>  virtio_input_init_config(vinput, virtio_keyboard_config);
> -virtio_input_key_config(vinput, qemu_input_map_qcode_to_linux,
> -qemu_input_map_qcode_to_linux_len);
> +virtio_input_extend_config(vinput, qemu_input_map_qcode_to_linux,
> +   qemu_input_map_qcode_to_linux_len,
> +   VIRTIO_INPUT_CFG_EV_BITS, EV_KEY);
>  }
>
>  static const TypeInfo virtio_keyboard_info = {
> @@ -373,8 +375,9 @@ static void virtio_mouse_init(Object *obj)
>  virtio_input_init_config(vinput, vhid->wheel_axis
>   ? virtio_mouse_config_v2
>   : virtio_mouse_config_v1);
> -virtio_input_key_config(vinput, keymap_button,
> -ARRAY_SIZE(keymap_button));
> +virtio_input_extend_config(vinput, keymap_button,
> +   ARRAY_SIZE(keymap_button),
> +   VIRTIO_INPUT_CFG_EV_BITS, EV_KEY);
>  }
>
>  static const TypeInfo virtio_mouse_info = {
> @@ -497,8 +500,9 @@ static void virtio_tablet_init(Object *obj)
>  virtio_input_init_config(vinput, vhid->wheel_axis
>   ? virtio_tablet_config_v2
>   : virtio_tablet_config_v1);
> -virtio_input_key_config(vinput, keymap_button,
> -ARRAY_SIZE(keymap_button));
> +virtio_input_extend_config(vinput, keymap_button,
> +   ARRAY_SIZE(keymap_button),
> +   VIRTIO_INPUT_CFG_EV_BITS, EV_KEY);
>  }
>
>  static const TypeInfo virtio_tablet_info = {
> --
> 2.38.1
>
>


--
Marc-André Lureau

Re: [PATCH 3/4] ui: add helpers for virtio-multitouch events

[Marc-André Lureau]

On Sat, Feb 18, 2023 at 8:23 PM Sergio Lopez  wrote:
>
> Add helpers for generating Multi-touch events from the UI backends that
> can be sent to the guest through a virtio-multitouch device.
>
> Signed-off-by: Sergio Lopez 

Reviewed-by: Marc-André Lureau 


> ---
>  include/ui/input.h |  5 +
>  ui/input.c | 36 
>  2 files changed, 41 insertions(+)
>
> diff --git a/include/ui/input.h b/include/ui/input.h
> index 2a3dffd417..c37251e1e9 100644
> --- a/include/ui/input.h
> +++ b/include/ui/input.h
> @@ -64,6 +64,11 @@ int qemu_input_scale_axis(int value,
>  void qemu_input_queue_rel(QemuConsole *src, InputAxis axis, int value);
>  void qemu_input_queue_abs(QemuConsole *src, InputAxis axis, int value,
>int min_in, int max_in);
> +void qemu_input_queue_mtt(QemuConsole *src, InputMultitouchType type, int 
> slot,
> +  int tracking_id);
> +void qemu_input_queue_mtt_abs(QemuConsole *src, InputAxis axis, int value,
> +  int min_in, int max_in,
> +  int slot, int tracking_id);
>
>  void qemu_input_check_mode_change(void);
>  void qemu_add_mouse_mode_change_notifier(Notifier *notify);
> diff --git a/ui/input.c b/ui/input.c
> index f788db20f7..34331b7b0b 100644
> --- a/ui/input.c
> +++ b/ui/input.c
> @@ -547,6 +547,42 @@ void qemu_input_queue_abs(QemuConsole *src, InputAxis 
> axis, int value,
>  qemu_input_event_send(src, );
>  }
>
> +void qemu_input_queue_mtt(QemuConsole *src, InputMultitouchType type,
> +  int slot, int tracking_id)
> +{
> +InputMultitouchEvent mtt = {
> +.type = type,
> +.slot = slot,
> +.tracking_id = tracking_id,
> +};
> +InputEvent evt = {
> +.type = INPUT_EVENT_KIND_MTT,
> +.u.mtt.data = ,
> +};
> +
> +qemu_input_event_send(src, );
> +}
> +
> +void qemu_input_queue_mtt_abs(QemuConsole *src, InputAxis axis, int value,
> +  int min_in, int max_in, int slot, int 
> tracking_id)
> +{
> +InputMultitouchEvent mtt = {
> +.type = INPUT_MULTITOUCH_TYPE_DATA,
> +.slot = slot,
> +.tracking_id = tracking_id,
> +.axis = axis,
> +.value = qemu_input_scale_axis(value, min_in, max_in,
> +   INPUT_EVENT_ABS_MIN,
> +   INPUT_EVENT_ABS_MAX),
> +};
> +InputEvent evt = {
> +.type = INPUT_EVENT_KIND_MTT,
> +.u.mtt.data = ,
> +};
> +
> +qemu_input_event_send(src, );
> +}
> +
>  void qemu_input_check_mode_change(void)
>  {
>  static int current_is_absolute;
> --
> 2.38.1
>
>


-- 
Marc-André Lureau

Re: [PATCH 2/4] virtio-input: add a virtio-mulitouch device

[Marc-André Lureau]

Hi

On Sat, Feb 18, 2023 at 8:23 PM Sergio Lopez  wrote:
>
> Add a virtio-multitouch device to the family of devices emulated by
> virtio-input implementing the Multi-touch protocol as descripted here:
>
> https://www.kernel.org/doc/html/latest/input/multi-touch-protocol.html?highlight=multi+touch
>
> This patch just add the device itself, without connecting it to any
> backends. The following patches will add helpers in "ui" and will enable
> the GTK3 backend to transpose multi-touch events from the host to the
> guest.
>

You should make the ui/ part as a different patch:
 qapi/ui.json  | 45 ++---
 include/ui/input.h|  3 +++
 replay/replay-input.c | 18 ++
 ui/input.c|  6 ++
 ui/trace-events   |  1 +

Similarly, I guess you could add the PCI device after, although that's
not as important.

> Signed-off-by: Sergio Lopez 
> ---
>  hw/input/virtio-input-hid.c  | 123 ++-
>  hw/virtio/virtio-input-pci.c |  25 ++-
>  include/hw/virtio/virtio-input.h |   9 ++-
>  include/ui/input.h   |   3 +
>  qapi/ui.json |  45 ++-
>  replay/replay-input.c|  18 +
>  ui/input.c   |   6 ++
>  ui/trace-events  |   1 +
>  8 files changed, 216 insertions(+), 14 deletions(-)
>
> diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
> index d28dab69ba..34109873ac 100644
> --- a/hw/input/virtio-input-hid.c
> +++ b/hw/input/virtio-input-hid.c
> @@ -16,9 +16,11 @@
>
>  #include "standard-headers/linux/input.h"
>
> -#define VIRTIO_ID_NAME_KEYBOARD "QEMU Virtio Keyboard"
> -#define VIRTIO_ID_NAME_MOUSE"QEMU Virtio Mouse"
> -#define VIRTIO_ID_NAME_TABLET   "QEMU Virtio Tablet"
> +#define VIRTIO_ID_NAME_KEYBOARD "QEMU Virtio Keyboard"
> +#define VIRTIO_ID_NAME_MOUSE"QEMU Virtio Mouse"
> +#define VIRTIO_ID_NAME_TABLET   "QEMU Virtio Tablet"
> +#define VIRTIO_ID_NAME_MULTITOUCH   "QEMU Virtio Multitouch"
> +#define VIRTIO_ID_SERIAL_MULTITOUCH "virtio-touchscreen-0"
>
>  /* - */
>
> @@ -30,6 +32,7 @@ static const unsigned short 
> keymap_button[INPUT_BUTTON__MAX] = {
>  [INPUT_BUTTON_WHEEL_DOWN]= BTN_GEAR_DOWN,
>  [INPUT_BUTTON_SIDE]  = BTN_SIDE,
>  [INPUT_BUTTON_EXTRA] = BTN_EXTRA,
> +[INPUT_BUTTON_TOUCH] = BTN_TOUCH,
>  };
>
>  static const unsigned short axismap_rel[INPUT_AXIS__MAX] = {
> @@ -42,6 +45,11 @@ static const unsigned short axismap_abs[INPUT_AXIS__MAX] = 
> {
>  [INPUT_AXIS_Y]   = ABS_Y,
>  };
>
> +static const unsigned short axismap_tch[INPUT_AXIS__MAX] = {
> +[INPUT_AXIS_X]   = ABS_MT_POSITION_X,
> +[INPUT_AXIS_Y]   = ABS_MT_POSITION_Y,
> +};
> +
>  /* - */
>
>  static void virtio_input_extend_config(VirtIOInput *vinput,
> @@ -81,6 +89,7 @@ static void virtio_input_handle_event(DeviceState *dev, 
> QemuConsole *src,
>  InputKeyEvent *key;
>  InputMoveEvent *move;
>  InputBtnEvent *btn;
> +InputMultitouchEvent *mtt;
>
>  switch (evt->type) {
>  case INPUT_EVENT_KIND_KEY:
> @@ -137,6 +146,24 @@ static void virtio_input_handle_event(DeviceState *dev, 
> QemuConsole *src,
>  event.value = cpu_to_le32(move->value);
>  virtio_input_send(vinput, );
>  break;
> +case INPUT_EVENT_KIND_MTT:
> +mtt = evt->u.mtt.data;
> +if (mtt->type == INPUT_MULTITOUCH_TYPE_DATA) {
> +event.type  = cpu_to_le16(EV_ABS);
> +event.code  = cpu_to_le16(axismap_tch[mtt->axis]);
> +event.value = cpu_to_le32(mtt->value);
> +virtio_input_send(vinput, );
> +} else {
> +event.type  = cpu_to_le16(EV_ABS);
> +event.code  = cpu_to_le16(ABS_MT_SLOT);
> +event.value = cpu_to_le32(mtt->slot);
> +virtio_input_send(vinput, );
> +event.type  = cpu_to_le16(EV_ABS);
> +event.code  = cpu_to_le16(ABS_MT_TRACKING_ID);
> +event.value = cpu_to_le32(mtt->tracking_id);
> +virtio_input_send(vinput, );
> +}
> +break;
>  default:
>  /* keep gcc happy */
>  break;
> @@ -515,12 +542,102 @@ static const TypeInfo virtio_tablet_info = {
>
>  /* - */
>
> +static QemuInputHandler virtio_multitouch_handler = {
> +.name  = VIRTIO_ID_NAME_MULTITOUCH,
> +.mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_MTT,
> +.event = virtio_input_handle_event,
> +.sync  = virtio_input_handle_sync,
> +};
> +
> +static struct virtio_input_config virtio_multitouch_config[] = {
> +{
> +.select= VIRTIO_INPUT_CFG_ID_NAME,
> +.size  =