qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH qemu v2 1/2] ppc: Define SETFIELD for the ppc target


From: Daniel Henrique Barboza
Subject: Re: [PATCH qemu v2 1/2] ppc: Define SETFIELD for the ppc target
Date: Mon, 27 Jun 2022 14:37:25 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0



On 6/27/22 01:54, Alexey Kardashevskiy wrote:


On 6/25/22 06:12, Daniel Henrique Barboza wrote:
Alexey,

The newer version of this patch is having trouble with Gitlab runners, as
you can read in my feedback there.

I've tested this one just in case. The same problems happen. E.g. for the
cross-armel-system runner:


In file included from ../hw/intc/pnv_xive.c:14:
../hw/intc/pnv_xive.c: In function ‘pnv_xive_block_id’:
/builds/danielhb/qemu/target/ppc/cpu.h:45:33: error: conversion from ‘long long 
unsigned int’ to ‘long unsigned int’ changes value from ‘4222124650659840’ to 
‘0’ [-Werror=overflow]
    45 | #define PPC_BITMASK(bs, be)     ((PPC_BIT(bs) - PPC_BIT(be)) | 
PPC_BIT(bs))
       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builds/danielhb/qemu/target/ppc/cpu.h:51:42: note: in definition of macro 
‘GETFIELD’
    51 |     (((word) & (mask)) >> __builtin_ctzl(mask))
       |                                          ^~~~
../hw/intc/pnv_xive_regs.h:77:41: note: in expansion of macro ‘PPC_BITMASK’
    77 | #define  PC_TCTXT_CHIPID                PPC_BITMASK(12, 15)
       |                                         ^~~~~~~~~~~
../hw/intc/pnv_xive.c:80:24: note: in expansion of macro ‘PC_TCTXT_CHIPID’
    80 |         blk = GETFIELD(PC_TCTXT_CHIPID, cfg_val);
       |                        ^~~~~~~~~~~~~~~
../hw/intc/pnv_xive.c: In function ‘pnv_xive_vst_addr’:
/builds/danielhb/qemu/target/ppc/cpu.h:45:33: error: conversion from ‘long long 
unsigned int’ to ‘long unsigned int’ changes value from ‘13835058055282163712’ 
to ‘0’ [-Werror=overflow]
    45 | #define PPC_BITMASK(bs, be)     ((PPC_BIT(bs) - PPC_BIT(be)) | 
PPC_BIT(bs))
       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builds/danielhb/qemu/target/ppc/cpu.h:51:42: note: in definition of macro 
‘GETFIELD’
    51 |     (((word) & (mask)) >> __builtin_ctzl(mask))
       |                                          ^~~~
../hw/intc/pnv_xive_regs.h:230:33: note: in expansion of macro ‘PPC_BITMASK’
   230 | #define VSD_MODE                PPC_BITMASK(0, 1)
       |                                 ^~~~~~~~~~~
../hw/intc/pnv_xive.c:226:18: note: in expansion of macro ‘VSD_MODE’
   226 |     if (GETFIELD(VSD_MODE, vsd) == VSD_MODE_FORWARD) {
       |                  ^~~~~~~~
../hw/intc/pnv_xive.c: In function ‘pnv_xive_end_update’:


Link:

https://gitlab.com/danielhb/qemu/-/jobs/2637716673


I don´t know how to deal with that.


For the record: if this is too troublesome to fix, I am ok with just 
consolidating
the GETFIELD and SETFIELD inlines we already have, under cpu.h, keeping them 
exactly
as they are today (functions, not macros).


Thanks,


Daniel



On 6/17/22 03:07, Alexey Kardashevskiy wrote:
It keeps repeating, move it to the header. This uses __builtin_ctzl() to
allow using the macros in #define.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
  include/hw/pci-host/pnv_phb3_regs.h | 16 ----------------
  target/ppc/cpu.h                    |  5 +++++
  hw/intc/pnv_xive.c                  | 20 --------------------
  hw/intc/pnv_xive2.c                 | 20 --------------------
  hw/pci-host/pnv_phb4.c              | 16 ----------------
  5 files changed, 5 insertions(+), 72 deletions(-)

diff --git a/include/hw/pci-host/pnv_phb3_regs.h 
b/include/hw/pci-host/pnv_phb3_regs.h
index a174ef1f7045..38f8ce9d7406 100644
--- a/include/hw/pci-host/pnv_phb3_regs.h
+++ b/include/hw/pci-host/pnv_phb3_regs.h
@@ -12,22 +12,6 @@
  #include "qemu/host-utils.h"
-/*
- * QEMU version of the GETFIELD/SETFIELD macros
- *
- * These are common with the PnvXive model.
- */
-static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
-{
-    return (word & mask) >> ctz64(mask);
-}
-
-static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
-                                uint64_t value)
-{
-    return (word & ~mask) | ((value << ctz64(mask)) & mask);
-}
-
  /*
   * PBCQ XSCOM registers
   */
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 6d78078f379d..9a1f1e9999a3 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -47,6 +47,11 @@
                                   PPC_BIT32(bs))
  #define PPC_BITMASK8(bs, be)    ((PPC_BIT8(bs) - PPC_BIT8(be)) | PPC_BIT8(bs))
+#define GETFIELD(mask, word)   \
+    (((word) & (mask)) >> __builtin_ctzl(mask))


Replacing __builtin_ctzl with __builtin_ctzll seems fixing it though, do you 
have a quick way to test this? Gitlab's CI takes time :)
https://gitlab.com/aik1/qemu/-/pipelines/573497191 is the current run.


I'll take a look, but apparently it fixed the problem I reported up above.

Also, there's an error in the msys2-64bit runner that I keep seeing from time to
time. It goes away eventually if you keep retrying it:


[301/1664] Generating input-keymap-qcode-to-qnum.c.inc with a custom command 
(wrapped by meson to capture output)
[302/1664] Generating input-keymap-qcode-to-sun.c.inc with a custom command 
(wrapped by meson to capture output)
[303/1664] Generating input-keymap-qnum-to-qcode.c.inc with a custom command 
(wrapped by meson to capture output)
[304/1664] Generating input-keymap-usb-to-qcode.c.inc with a custom command 
(wrapped by meson to capture output)
FAILED: ui/input-keymap-usb-to-qcode.c.inc
"C:/GitLab-Runner/builds/aik1/qemu/msys64/mingw64/bin/python3.exe" "C:/GitLab-Runner/builds/aik1/qemu/meson/meson.py" "--internal" "exe" "--capture" "ui/input-keymap-usb-to-qcode.c.inc" 
"--" "C:/GitLab-Runner/builds/aik1/qemu/msys64/mingw64/bin/python3.exe" "../ui/keycodemapdb/tools/keymap-gen" "code-map" "--lang" "glib2" "--varname" 
"qemu_input_map_usb_to_qcode" "../ui/keycodemapdb/data/keymaps.csv" "usb" "qcode"
[305/1664] Generating input-keymap-win32-to-qcode.c.inc with a custom command 
(wrapped by meson to capture output)
ninja: build stopped: subcommand failed.
make[1]: Leaving directory '/c/GitLab-Runner/builds/aik1/qemu/build'
make[1]: *** [Makefile:162: run-ninja] Error 1
make: *** [GNUmakefile:11: all] Error 2


It's a strange one because it's an error triggered by an ui/keymap file which 
you're
not changing.

Richard already created a thread about it in the QEMU ML, so I'll assume that
this has nothing to do with powerpc code.


Thanks,

Daniel


Thanks,





reply via email to

[Prev in Thread] Current Thread [Next in Thread]