[Bug 1891748] Re: qemu-arm-static 5.1 can't run gcc

[xornet]

Can confirm this bug on fresh Linux Arch and Debian Linux installation.
I need just nothing to reproduce it: Just install fresh arch and do steps 
described in comment #1

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1891748

Title:
  qemu-arm-static 5.1 can't run gcc

Status in QEMU:
  Fix Released
Status in Juju Charms Collection:
  New

Bug description:
  Issue discovered while trying to build pikvm (1)

  Long story short: when using qemu-arm-static 5.1, gcc exits whith
  message:

  Allocating guest commpage: Operation not permitted

  
  when using qemu-arm-static v5.0, gcc "works"

  Steps to reproduce will follow

  (1)  https://github.com/pikvm/pikvm/blob/master/pages/building_os.md

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1891748/+subscriptions

Re: Bug in qemu-system-ppc running fedora 12 ppc guest

[Howard Spoelstra]

On Thu, Jul 29, 2021 at 12:07 PM BALATON Zoltan  wrote:

> Hello,
>
> On Thu, 29 Jul 2021, Howard Spoelstra wrote:
> > Hi,
> >
> > Qemu-system-ppc built from current master can no longer run Fedora 12 ppc
> > as guest. (This the only ppc distro I tested.) Host is Fedora 34. Please
> > see screen shot attached.
> > Booting from both an installation DVD and from an installed system fail.
> >
> > To reproduce:
> > compile qemu-system-ppc from current master and run:
> >
> > ./qemu-system-ppc \
> > -M mac99,via=pmu \
> > -m 1024 \
> > -L pc-bios \
> > -boot d \
> > -cdrom Fedora-12-ppc-DVD.iso \
> > -g 1024x768x8
> >
> > I tracked the issue down to this commit:
> >
> > 8f0a4b6a9b40e18116a2bb6bbcc00feb8119c792 is the first bad commit
> > commit 8f0a4b6a9b40e18116a2bb6bbcc00feb8119c792
>
> There's a fix for a similar problem I've seen with AROS and pegasos2
> firmware 1.2 that's in today's pull request:
>
> https://lists.nongnu.org/archive/html/qemu-ppc/2021-07/msg00281.html
>
> That should likely fixes this. Can you try with that (either once it's
> merged or from David's for-6.1 branch).
>
> Regards,
> BALATON Zoltan
>

Thanks, this issue is indeed fixed by
https://github.com/qemu/qemu/commit/2d1154bd95a8bfea30cc59de8e080e5a016a9bee

Best,
Howard

[PATCH 1/1] hw/arm/smmu: Add access flag handling

[Joe Komlodi]

The SMMU should access fault if AF == 0 in a TTD, and if AFFD == 0 in the CD.

Per the spec, an access fault has a higher priority over a permission fault.
For instance, a write to a writable clean (temporarily non-writable) page with
AF == 0 && AFFD == 0 will cause an access fault.
If AF == 1 in this situation, then a permission fault would occur.

Access flag handling is more complicated if HTTU is supported and HA != 0 in
the CD, however we currently do not support HTTU.

Signed-off-by: Joe Komlodi 
---
 hw/arm/smmu-common.c | 7 +++
 hw/arm/smmu-internal.h   | 8 
 hw/arm/smmuv3-internal.h | 1 +
 hw/arm/smmuv3.c  | 1 +
 include/hw/arm/smmu-common.h | 1 +
 5 files changed, 18 insertions(+)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 0459850..0fcc65c 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -305,6 +305,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
 uint64_t pte, gpa;
 dma_addr_t pte_addr = baseaddr + offset * sizeof(pte);
 uint8_t ap;
+bool af;
 
 if (get_pte(baseaddr, offset, , info)) {
 goto error;
@@ -341,6 +342,12 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
  pte_addr, pte, iova, gpa,
  block_size >> 20);
 }
+af = PTE_AF(pte);
+if (is_access_fault(af, perm)) {
+info->type = SMMU_PTW_ERR_ACCESS;
+goto error;
+}
+
 ap = PTE_AP(pte);
 if (is_permission_fault(ap, perm)) {
 info->type = SMMU_PTW_ERR_PERMISSION;
diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
index 2d75b31..9d3b22c 100644
--- a/hw/arm/smmu-internal.h
+++ b/hw/arm/smmu-internal.h
@@ -58,6 +58,11 @@
 ((level == 3) &&\
  ((pte & ARM_LPAE_PTE_TYPE_MASK) == ARM_LPAE_L3_PTE_TYPE_PAGE))
 
+/* access flag */
+
+#define PTE_AF(pte) \
+(extract64(pte, 10, 1))
+
 /* access permissions */
 
 #define PTE_AP(pte) \
@@ -66,6 +71,9 @@
 #define PTE_APTABLE(pte) \
 (extract64(pte, 61, 2))
 
+#define is_access_fault(af, cfg) \
+(!cfg->affd && !af)
+
 /*
  * TODO: At the moment all transactions are considered as privileged (EL1)
  * as IOMMU translation callback does not pass user/priv attributes.
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index d1885ae..0ccad1d 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -587,6 +587,7 @@ static inline int pa_range(STE *ste)
 #define CD_EPD(x, sel)   extract32((x)->word[0], (16 * (sel)) + 14, 1)
 #define CD_ENDI(x)   extract32((x)->word[0], 15, 1)
 #define CD_IPS(x)extract32((x)->word[1], 0 , 3)
+#define CD_AFFD(x)   extract32((x)->word[1], 3 , 1)
 #define CD_TBI(x)extract32((x)->word[1], 6 , 2)
 #define CD_HD(x) extract32((x)->word[1], 10 , 1)
 #define CD_HA(x) extract32((x)->word[1], 11 , 1)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 01b60be..df5a194 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -483,6 +483,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, 
SMMUEventInfo *event)
 cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas);
 cfg->tbi = CD_TBI(cd);
 cfg->asid = CD_ASID(cd);
+cfg->affd = CD_AFFD(cd);
 
 trace_smmuv3_decode_cd(cfg->oas);
 
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 706be3c..b0e82ad 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -71,6 +71,7 @@ typedef struct SMMUTransCfg {
 bool disabled; /* smmu is disabled */
 bool bypassed; /* translation is bypassed */
 bool aborted;  /* translation is aborted */
+bool affd; /* Access Flag Fault Disabled */
 uint64_t ttb;  /* TT base address */
 uint8_t oas;   /* output address width */
 uint8_t tbi;   /* Top Byte Ignore */
-- 
2.7.4

[PATCH 0/1] hw/arm/smmu: Add access flag handling

[Joe Komlodi]

Hi all,

This adds a check in SMMU PTW to see if the access flag bit is set in a PTE, and
if we should fault accordingly or not.

Since we do not support HTTU, the check itself is pretty simple:
If AFFD == 0 in the context descriptor and AF == 0 in the PTE, we fault.
Otherwise, we do not have an access fault.

Thanks!
Joe

Joe Komlodi (1):
  hw/arm/smmu: Add access flag handling

 hw/arm/smmu-common.c | 7 +++
 hw/arm/smmu-internal.h   | 8 
 hw/arm/smmuv3-internal.h | 1 +
 hw/arm/smmuv3.c  | 1 +
 include/hw/arm/smmu-common.h | 1 +
 5 files changed, 18 insertions(+)

-- 
2.7.4

[PATCH] target/mips: Remove JR opcode unused arguments

[Philippe Mathieu-Daudé]

JR opcode (Jump Register) only takes 1 argument, $rs.
JALR (Jump And Link Register) takes 3: $rs, $rd and $hint.

Commit 6af0bf9c7c3 added their processing into decode_opc() as:

case 0x08 ... 0x09: /* Jumps */
gen_compute_branch(ctx, op1 | EXT_SPECIAL, rs, rd, sa);

having both opcodes handled in the same function: gen_compute_branch.

Per JR encoding, both $rd and $hint ('sa') are decoded as zero.

Later this code got extracted to decode_opc_special(),
commit 7a387fffce5 used definitions instead of magic values:

case OPC_JR ... OPC_JALR:
gen_compute_branch(ctx, op1, rs, rd, sa);

Finally commit 0aefa33318b moved OPC_JR out of decode_opc_special,
to a new 'decode_opc_special_legacy' function:

  @@ -15851,6 +15851,9 @@ static void decode_opc_special_legacy(CPUMIPSState 
*env, DisasContext *ctx)
  +case OPC_JR:
  +gen_compute_branch(ctx, op1, 4, rs, rd, sa);
  +break;

  @@ -15933,7 +15936,7 @@ static void decode_opc_special(CPUMIPSState *env, 
DisasContext *ctx)
  -case OPC_JR ... OPC_JALR:
  +case OPC_JALR:
   gen_compute_branch(ctx, op1, 4, rs, rd, sa);
   break;

Since JR is now handled individually, it is pointless to decode
and pass it unused arguments. Replace them by simple zero value
to avoid confusion with this opcode.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/tcg/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 5b03545f099..bf71724f3f0 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -14203,7 +14203,7 @@ static void decode_opc_special_legacy(CPUMIPSState 
*env, DisasContext *ctx)
 break;
 #endif
 case OPC_JR:
-gen_compute_branch(ctx, op1, 4, rs, rd, sa, 4);
+gen_compute_branch(ctx, op1, 4, rs, 0, 0, 4);
 break;
 case OPC_SPIM:
 #ifdef MIPS_STRICT_STANDARD
-- 
2.31.1

Re: "make check-acceptance" takes way too long

[Cleber Rosa]

On Fri, Jul 30, 2021 at 11:43 AM Peter Maydell  wrote:
>
> On Fri, 30 Jul 2021 at 16:12, Peter Maydell  wrote:
> >
> > "make check-acceptance" takes way way too long. I just did a run
> > on an arm-and-aarch64-targets-only debug build and it took over
> > half an hour, and this despite it skipping or cancelling 26 out
> > of 58 tests!
> >
> > I think that ~10 minutes runtime is reasonable. 30 is not;
> > ideally no individual test would take more than a minute or so.
>
> Side note, can check-acceptance run multiple tests in parallel?

Yes, it can, but it's not currently enabled to do so, but I'm planning
to.  As a matter of fact, Yesterday I was trying out Avocado's
parallel capable runner on a GitLab CI pipeline[1] and it went well.

> Running 3 or 4 at once would also improve the runtime...
>

About the time savings, on my own machine I see good results.  On a
build with only the x86_64 target, the parallel execution gets me:

$ avocado run -t arch:x86_64 --filter-by-tags-include-empty
--filter-by-tags-include-empty-key --test-runner=nrunner
--nrunner-max-parallel-tasks=4 tests/acceptance/
...
RESULTS: PASS 37 | ERROR 0 | FAIL 0 | SKIP 6 | WARN 5 | INTERRUPT
0 | CANCEL 0
...
JOB TIME   : 244.59 s

While the serial execution gets me:

$ avocado run -t arch:x86_64 --filter-by-tags-include-empty
--filter-by-tags-include-empty-key tests/acceptance/
...
RESULTS: PASS 37 | ERROR 0 | FAIL 0 | SKIP 6 | WARN 5 | INTERRUPT
0 | CANCEL 0
...
JOB TIME   : 658.65 s

But the environment on GitLab CI is fluid, and I bet there's already
some level of overcommit of (at least) CPUs there.  The only pipeline
I ran there with tests running in parallel, resulted in some jobs with
improvements, and others with regressions in runtime.  Additionally,
lack of adequate resources can make more tests time out, and thus give
out false negatives.

Anyway, my current plan is to allow users to configure the
parallelization level on their machines, while slowly and steadily
experimenting what can safely improve the runtime on GitLab CI.

BTW, another **very** sweet spot, which I have experimented with
before, is letting Avocado run the acceptance tests and the iotests in
parallel because they compete for pretty much different resources.
But, that's a matter for another round.

> -- PMM
>

Best regards,
- Cleber.

[1] https://gitlab.com/cleber.gnu/qemu/-/pipelines/344471529
[2] https://gitlab.com/cleber.gnu/qemu/-/pipelines/345082239

Re: [PATCH for-6.2 53/53] target/arm: Enable MVE in Cortex-M55

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

We now have a complete MVE emulation, so we can enable it in our
Cortex-M55 model by setting the ID registers to match those of a
Cortex-M55 with full MVE support.

Signed-off-by: Peter Maydell
---
  docs/system/arm/emulation.rst | 1 +
  target/arm/cpu_tcg.c  | 7 ++-
  2 files changed, 3 insertions(+), 5 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 52/53] target/arm: Implement MVE VRINT insns

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE VRINT insns, which round floating point inputs
to integer values, leaving them in floating point format.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  6 +
  target/arm/mve.decode  |  7 ++
  target/arm/mve_helper.c| 35 +
  target/arm/translate-mve.c | 45 ++
  4 files changed, 93 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 12/43] target/sh4: Implement do_unaligned_access for user-only

[Rob Landley]

On 7/29/21 8:52 AM, Peter Maydell wrote:
> On Thu, 29 Jul 2021 at 02:01, Richard Henderson
>  wrote:
>>
>> Cc: Yoshinori Sato 
>> Signed-off-by: Richard Henderson 
>> ---
>>  linux-user/sh4/cpu_loop.c | 8 
>>  target/sh4/cpu.c  | 2 +-
>>  target/sh4/op_helper.c| 3 ---
>>  3 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/linux-user/sh4/cpu_loop.c b/linux-user/sh4/cpu_loop.c
>> index 222ed1c670..21d97250a8 100644
>> --- a/linux-user/sh4/cpu_loop.c
>> +++ b/linux-user/sh4/cpu_loop.c
>> @@ -71,6 +71,14 @@ void cpu_loop(CPUSH4State *env)
>>  info._sifields._sigfault._addr = env->tea;
>>  queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
>>  break;
>> +case 0xe0:
>> +case 0x100:
>> +info.si_signo = TARGET_SIGBUS;
>> +info.si_errno = 0;
>> +info.si_code = TARGET_BUS_ADRALN;
>> +info._sifields._sigfault._addr = env->tea;
>> +queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
>> +break;
> 
> sh4 kernel default for unaligned accesses seems to be "warn and fixup",
> not SIGBUS, unless the user changes that by writing to /proc/cpu/alignment
> or the process changes it via prctl().

It's still good to know, qemu-sh4 runs j-core binaries but that target doesn't
have unaligned interrupts yet. (I think it just masks off the bottom 2 bits to
do the next lowest aligned access? It's an sh2 variant and the plumbing to let
interrupts restart multi-clock instructions is only in the j32 branch so far, so
the j2 and ice40 targets don't generate interrupts for it. Todo item, in the
meantime we need to clean unaligned access out of application code so faulting
on it is good.)

> -- PMM

Thanks,

Rob

Re: [PATCH for-6.2 51/53] target/arm: Implement MVE VCVT between single and half precision

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

+/*
+ * VCVT between halfprec and singleprec. As usual for halfprec
+ * conversions, FZ16 is ignored and AHP is observed.
+ */
+#define DO_VCVT_SH(OP, TOP) \
+void HELPER(glue(mve_, OP))(CPUARMState *env, void *vd, void *vm)   \


These might be easier to read as a local function with an extra "top" argument, rather 
than as a macro, since you're not passing in anything complicated.


Otherwise,
Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 50/53] target/arm: Implement MVE VCVT with specified rounding mode

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE VCVT which converts from floating-point to integer
using a rounding mode specified by the instruction.  We implement
this similarly to the Neon equivalents, by passing the required
rounding mode as an extra integer parameter to the helper functions.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  5 
  target/arm/mve.decode  | 10 
  target/arm/mve_helper.c| 38 
  target/arm/translate-mve.c | 52 ++
  4 files changed, 105 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 48/53] target/arm: Implement MVE VCVT between floating and fixed point

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE VCVT insns which convert between floating and fixed
point.  As with the Neon equivalents, these use essentially the same
constant encoding as right-shift-by-immediate.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  9 +
  target/arm/mve.decode  | 19 +++
  target/arm/mve_helper.c| 36 
  target/arm/translate-mve.c | 18 ++
  4 files changed, 82 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 49/53] target/arm: Implement MVE VCVT between fp and integer

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE "VCVT (between floating-point and integer)" insn.

Signed-off-by: Peter Maydell
---
  target/arm/mve.decode  |  7 +++
  target/arm/translate-mve.c | 32 
  2 files changed, 39 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 46/53] target/arm: Implement MVE fp vector comparisons

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE fp vector comparisons VCMP and VPT.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h| 18 +++
  target/arm/mve.decode  | 39 +++
  target/arm/mve_helper.c| 64 ++
  target/arm/translate-mve.c | 22 +
  4 files changed, 137 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PATCH v3 13/13] python/aqmp-tui: Allow copying message from TUI

[G S Niteesh Babu]

This commit adds a feature that enables use to copy
messages from the TUI after highlighting the message
in the history box using up/down arrow keys and pressing
alt-c.

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/aqmp_tui.py | 9 +
 1 file changed, 9 insertions(+)

diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
index 4bae0d4e89..434f431a35 100644
--- a/python/qemu/aqmp/aqmp_tui.py
+++ b/python/qemu/aqmp/aqmp_tui.py
@@ -21,6 +21,7 @@
 
 from pygments import lexers
 from pygments import token as Token
+import pyperclip
 import urwid
 import urwid_readline
 
@@ -390,6 +391,14 @@ def keypress(self, size: Tuple[int, int], key: str) -> 
Optional[str]:
 self._update_highlighting()
 self.change_focus(size, self.highlighting)
 return None
+if key == 'meta c':
+if self.highlighting == -1:
+return None
+widget = self.history[self.highlighting].original_widget
+text = widget.get_text()[0]
+LOGGER.info('Text is %s', text)
+pyperclip.copy(text)
+return None
 
 # Remove highlighting if someother key is pressed
 if self.highlighting != -1:
-- 
2.17.1

[PATCH v3 12/13] python/aqmp-tui: Add pyperclip dependency

[G S Niteesh Babu]

This dependency is required to enable copying from the TUI
using special keys to the system clipboard.

pyperclip works out of the box on windows and macos but requires
xsel/xclip to be installed on linux machines.

Signed-off-by: G S Niteesh Babu 
---
 python/Pipfile.lock | 22 ++
 python/setup.cfg|  5 +
 2 files changed, 27 insertions(+)

diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 2c6d779348..3544c8703d 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -45,6 +45,14 @@
 "index": "pypi",
 "version": "==87.0"
 },
+"backports.entry-points-selectable": {
+"hashes": [
+
"sha256:988468260ec1c196dab6ae1149260e2f5472c9110334e5d51adcb77867361f6a",
+
"sha256:a6d9a871cde5e15b4c4a53e3d43ba890cc6861ec1332c9c2428c92f977192acc"
+],
+"markers": "python_version >= '2.7'",
+"version": "==1.1.0"
+},
 "distlib": {
 "hashes": [
 
"sha256:106fef6dc37dd8c0e2c0a60d3fca3e77460a48907f335fa28420463a6f799736",
@@ -169,6 +177,14 @@
 "markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3'",
 "version": "==20.9"
 },
+"platformdirs": {
+"hashes": [
+
"sha256:4666d822218db6a262bdfdc9c39d21f23b4cfdb08af331a81e92751daf6c866c",
+
"sha256:632daad3ab546bd8e6af0537d09805cec458dce201bccfe23012df73332e181e"
+],
+"markers": "python_version >= '3.6'",
+"version": "==2.2.0"
+},
 "pluggy": {
 "hashes": [
 
"sha256:15b2acde666561e1298d71b523007ed7364de07029219b604cf808bfa1c765b0",
@@ -224,6 +240,12 @@
 "markers": "python_version >= '2.6' and python_version not in 
'3.0, 3.1, 3.2, 3.3'",
 "version": "==2.4.7"
 },
+"pyperclip": {
+"hashes": [
+
"sha256:105254a8b04934f0bc84e9c24eb360a591aaf6535c9def5f29d92af107a9bf57"
+],
+"version": "==1.8.2"
+},
 "qemu": {
 "editable": true,
 "path": "."
diff --git a/python/setup.cfg b/python/setup.cfg
index bbb7306c3d..683c0b1d00 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -47,6 +47,7 @@ devel =
 urwid >= 2.1.2
 urwid-readline >= 0.13
 Pygments >= 2.9.0
+pyperclip >= 1.8.2
 
 # Provides qom-fuse functionality
 fuse =
@@ -57,6 +58,7 @@ tui =
 urwid >= 2.1.2
 urwid-readline >= 0.13
 Pygments >= 2.9.0
+pyperclip >= 1.8.2
 
 [options.entry_points]
 console_scripts =
@@ -102,6 +104,9 @@ ignore_missing_imports = True
 [mypy-pygments]
 ignore_missing_imports = True
 
+[mypy-pyperclip]
+ignore_missing_imports = True
+
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
-- 
2.17.1

[PATCH v3 07/13] python: add optional pygments dependency

[G S Niteesh Babu]

Added pygments as optional dependency for AQMP TUI.
This is required for the upcoming syntax highlighting feature
in AQMP TUI.
The dependency has also been added in the devel optional group.

Added mypy 'ignore_missing_imports' for pygments since it does
not have any type stubs.

Signed-off-by: G S Niteesh Babu 
---
 python/Pipfile.lock | 8 
 python/setup.cfg| 5 +
 2 files changed, 13 insertions(+)

diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 76cf1e4930..2c6d779348 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -200,6 +200,14 @@
 ],
 "version": "==2.0.0"
 },
+"pygments": {
+"hashes": [
+
"sha256:a18f47b506a429f6f4b9df81bb02beab9ca21d0a5fee38ed15aef65f0545519f",
+
"sha256:d66e804411278594d764fc69ec36ec13d9ae9147193a1740cd34d272ca383b8e"
+],
+"markers": "python_version >= '3.5'",
+"version": "==2.9.0"
+},
 "pylint": {
 "hashes": [
 
"sha256:082a6d461b54f90eea49ca90fff4ee8b6e45e8029e5dbd72f6107ef84f3779c0",
diff --git a/python/setup.cfg b/python/setup.cfg
index 11c6240aba..bbb7306c3d 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -46,6 +46,7 @@ devel =
 tox >= 3.18.0
 urwid >= 2.1.2
 urwid-readline >= 0.13
+Pygments >= 2.9.0
 
 # Provides qom-fuse functionality
 fuse =
@@ -55,6 +56,7 @@ fuse =
 tui =
 urwid >= 2.1.2
 urwid-readline >= 0.13
+Pygments >= 2.9.0
 
 [options.entry_points]
 console_scripts =
@@ -97,6 +99,9 @@ ignore_missing_imports = True
 [mypy-urwid_readline]
 ignore_missing_imports = True
 
+[mypy-pygments]
+ignore_missing_imports = True
+
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
-- 
2.17.1

[PATCH v3 08/13] python/aqmp-tui: add syntax highlighting

[G S Niteesh Babu]

Add syntax highlighting for the incoming and outgoing QMP messages.
This is achieved using the pygments module which was added in a
previous commit.

The current implementation is a really simple one which doesn't
allow for any configuration. In future this has to be improved
to allow for easier theme config using an external config of
some sort.

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/aqmp_tui.py | 52 +++-
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
index ab9ada793a..0d5ec62cb7 100644
--- a/python/qemu/aqmp/aqmp_tui.py
+++ b/python/qemu/aqmp/aqmp_tui.py
@@ -19,6 +19,8 @@
 Union,
 )
 
+from pygments import lexers
+from pygments import token as Token
 import urwid
 import urwid_readline
 
@@ -35,6 +37,22 @@
 LOGGER = logging.getLogger()
 
 
+palette = [
+(Token.Punctuation, '', '', '', 'h15,bold', 'g7'),
+(Token.Text, '', '', '', '', 'g7'),
+(Token.Name.Tag, '', '', '', 'bold,#f88', 'g7'),
+(Token.Literal.Number.Integer, '', '', '', '#fa0', 'g7'),
+(Token.Literal.String.Double, '', '', '', '#6f6', 'g7'),
+(Token.Keyword.Constant, '', '', '', '#6af', 'g7'),
+('DEBUG', '', '', '', '#ddf', 'g7'),
+('INFO', '', '', '', 'g100', 'g7'),
+('WARNING', '', '', '', '#ff6', 'g7'),
+('ERROR', '', '', '', '#a00', 'g7'),
+('CRITICAL', '', '', '', '#a00', 'g7'),
+('background', '', 'black', '', '', 'g7'),
+]
+
+
 def format_json(msg: str) -> str:
 """
 Formats given multiline JSON message into a single line message.
@@ -57,17 +75,14 @@ def __init__(self, address: Union[str, Tuple[str, int]]) -> 
None:
 self.aloop: Optional[Any] = None  # FIXME: Use more concrete type.
 super().__init__()
 
-def add_to_history(self, msg: str) -> None:
-urwid.emit_signal(self, UPDATE_MSG, msg)
+def add_to_history(self, msg: str, level: Optional[str] = None) -> None:
+urwid.emit_signal(self, UPDATE_MSG, msg, level)
 
 def _cb_outbound(self, msg: Message) -> Message:
 # FIXME: I think the ideal way to omit these messages during in-TUI
-# logging will be to add a filter to the logger. We can use regex to
-# filter out messages starting with 'Request:' or 'Response:' but I
-# think a better approach will be encapsulate the message in an object
-# and filter based on the object. Encapsulation of the message will
-# also be necessary when we want different formatting of messages
-# inside TUI.
+# logging will be to add a filter to the logger. We can use
+# regex/startswith to filter out messages starting with 'Request:' or
+# 'Response:'. If possible please suggest other ideas.
 handler = LOGGER.handlers[0]
 if not isinstance(handler, TUILogHandler):
 LOGGER.debug('Request: %s', str(msg))
@@ -156,6 +171,9 @@ def _get_formatted_address(self) -> str:
 self._set_status('Server shutdown')
 
 def run(self, debug: bool = False) -> None:
+screen = urwid.raw_display.Screen()
+screen.set_terminal_properties(256)
+
 self.aloop = asyncio.get_event_loop()
 self.aloop.set_debug(debug)
 
@@ -167,6 +185,8 @@ def run(self, debug: bool = False) -> None:
 event_loop = urwid.AsyncioEventLoop(loop=self.aloop)
 main_loop = urwid.MainLoop(urwid.AttrMap(self.window, 'background'),
unhandled_input=self.unhandled_input,
+   screen=screen,
+   palette=palette,
handle_mouse=True,
event_loop=event_loop)
 
@@ -251,7 +271,8 @@ def __init__(self, master: App) -> None:
 self.history = urwid.SimpleFocusListWalker([])
 super().__init__(self.history)
 
-def add_to_history(self, history: str) -> None:
+def add_to_history(self,
+   history: Union[str, List[Tuple[str, str]]]) -> None:
 self.history.append(urwid.Text(history))
 if self.history:
 self.history.set_focus(len(self.history) - 1)
@@ -271,8 +292,15 @@ def __init__(self, master: App) -> None:
 super().__init__(self.body)
 urwid.connect_signal(self.master, UPDATE_MSG, self.cb_add_to_history)
 
-def cb_add_to_history(self, msg: str) -> None:
-self.history.add_to_history(msg)
+def cb_add_to_history(self, msg: str, level: Optional[str] = None) -> None:
+formatted = []
+if level:
+formatted.append((level, msg))
+else:
+lexer = lexers.JsonLexer()  # pylint: disable=no-member
+for token in lexer.get_tokens(msg):
+formatted.append(token)
+self.history.add_to_history(formatted)
 
 
 class Window(urwid.Frame):
@@ -298,7 +326,7 @@ def emit(self, record: LogRecord) -> None:
 

[PATCH v3 11/13] python/aqmp-tui: Add ability to highlight messages

[G S Niteesh Babu]

Adds ability to highlight messages in the history box. The messages
can be selected using up/down arrow keys.
This can be enhanced in the future to apply specific settings to
a particular message.

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/aqmp_tui.py | 50 
 1 file changed, 50 insertions(+)

diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
index fb828b1a27..4bae0d4e89 100644
--- a/python/qemu/aqmp/aqmp_tui.py
+++ b/python/qemu/aqmp/aqmp_tui.py
@@ -344,6 +344,7 @@ def __init__(self, master: App) -> None:
 self.master = master
 self.history = urwid.SimpleFocusListWalker([])
 super().__init__(self.history)
+self.highlighting = -1
 
 def add_to_history(self,
history: Union[str, List[Tuple[str, str]]]) -> None:
@@ -351,8 +352,57 @@ def add_to_history(self,
 if self.history:
 self.history.set_focus(len(self.history) - 1)
 
+def _remove_highlighting(self) -> None:
+assert self.highlighting != -1
+pos = self.highlighting
+widget = self.history[pos]
+widget = widget.original_widget
+self.history[pos] = widget
+
+def _update_highlighting(self) -> None:
+assert self.highlighting != -1
+pos = self.highlighting
+widget = self.history[pos]
+self.history[pos] = urwid.LineBox(widget)
+
+def keypress(self, size: Tuple[int, int], key: str) -> Optional[str]:
+if key == 'up':
+if self.highlighting != -1:
+pos = self.highlighting
+self._remove_highlighting()
+pos = max(pos - 1, 0)
+self.highlighting = pos
+else:
+self.highlighting = len(self.history) - 1
+self._update_highlighting()
+self.change_focus(size, self.highlighting)
+return None
+if key == 'down':
+pos = self.highlighting
+if pos == -1:
+return None
+
+self._remove_highlighting()
+if pos == len(self.history) - 1:
+self.highlighting = -1
+else:
+self.highlighting = pos + 1
+self._update_highlighting()
+self.change_focus(size, self.highlighting)
+return None
+
+# Remove highlighting if someother key is pressed
+if self.highlighting != -1:
+self._remove_highlighting()
+self.highlighting = -1
+return super().keypress(size, key)  # type: ignore
+
 def mouse_event(self, size: Tuple[int, int], _event: str, button: float,
 _x: int, _y: int, focus: bool) -> None:
+if self.highlighting != -1:
+self._remove_highlighting()
+self.highlighting = -1
+
 # Scroll only on focus. Therefore it is required to
 # click on the widget to enable scrolling.
 if not focus:
-- 
2.17.1

Re: [PATCH for-6.2 47/53] target/arm: Implement MVE fp scalar comparisons

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE fp scalar comparisons VCMP and VPT.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h| 18 +++
  target/arm/mve.decode  | 61 +
  target/arm/mve_helper.c| 62 ++
  target/arm/translate-mve.c | 14 +
  4 files changed, 131 insertions(+), 24 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PATCH v3 05/13] python: add entry point for aqmp-tui

[G S Niteesh Babu]

Add an entry point for aqmp-tui. This will allow it to be run from
the command line using "aqmp-tui localhost:1234"
More options available in the TUI can be found using "aqmp-tui -h"

Signed-off-by: G S Niteesh Babu 
---
 python/setup.cfg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 50f9894468..8cd9ac0d81 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -66,6 +66,7 @@ console_scripts =
 qom-fuse = qemu.qmp.qom_fuse:QOMFuse.entry_point [fuse]
 qemu-ga-client = qemu.qmp.qemu_ga_client:main
 qmp-shell = qemu.qmp.qmp_shell:main
+aqmp-tui = qemu.aqmp.aqmp_tui:main [tui]
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
-- 
2.17.1

[PATCH v3 03/13] python: Add dependencies for AQMP TUI

[G S Niteesh Babu]

Added dependencies for the upcoming AQMP TUI under the optional
'tui' group.

The same dependencies have also been added under the devel group
since no work around has been found for optional groups to imply
other optional groups.

Signed-off-by: G S Niteesh Babu 
---
 python/Pipfile.lock | 12 
 python/setup.cfg|  8 
 2 files changed, 20 insertions(+)

diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 8ab41a3f60..76cf1e4930 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -289,6 +289,18 @@
 "markers": "python_version < '3.8'",
 "version": "==3.10.0.0"
 },
+"urwid": {
+"hashes": [
+
"sha256:588bee9c1cb208d0906a9f73c613d2bd32c3ed3702012f51efe318a3f2127eae"
+],
+"version": "==2.1.2"
+},
+"urwid-readline": {
+"hashes": [
+
"sha256:018020cbc864bb5ed87be17dc26b069eae2755cb29f3a9c569aac3bded1efaf4"
+],
+"version": "==0.13"
+},
 "virtualenv": {
 "hashes": [
 
"sha256:14fdf849f80dbb29a4eb6caa9875d476ee2a5cf76a5f5415fa2f1606010ab467",
diff --git a/python/setup.cfg b/python/setup.cfg
index 7a30dd5b09..d106a0ed7a 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -44,11 +44,18 @@ devel =
 mypy >= 0.770
 pylint >= 2.8.0
 tox >= 3.18.0
+urwid >= 2.1.2
+urwid-readline >= 0.13
 
 # Provides qom-fuse functionality
 fuse =
 fusepy >= 2.0.4
 
+# AQMP TUI dependencies
+tui =
+urwid >= 2.1.2
+urwid-readline >= 0.13
+
 [options.entry_points]
 console_scripts =
 qom = qemu.qmp.qom:main
@@ -133,5 +140,6 @@ allowlist_externals = make
 deps =
 .[devel]
 .[fuse]  # Workaround to trigger tox venv rebuild
+.[tui]   # Workaround to trigger tox venv rebuild
 commands =
 make check
-- 
2.17.1

[PATCH v3 10/13] python/aqmp-tui: Add scrolling to history box

[G S Niteesh Babu]

Adds scroll support to history box. The list can now be scrolled
using arrow keys, page up/down and the mouse.

The current implementation requires the widget to be in focus
to enable scrolling. Therefore the user has to click on the widget
before scrolling.

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/aqmp_tui.py | 13 +
 1 file changed, 13 insertions(+)

diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
index ef91883fa5..fb828b1a27 100644
--- a/python/qemu/aqmp/aqmp_tui.py
+++ b/python/qemu/aqmp/aqmp_tui.py
@@ -351,6 +351,19 @@ def add_to_history(self,
 if self.history:
 self.history.set_focus(len(self.history) - 1)
 
+def mouse_event(self, size: Tuple[int, int], _event: str, button: float,
+_x: int, _y: int, focus: bool) -> None:
+# Scroll only on focus. Therefore it is required to
+# click on the widget to enable scrolling.
+if not focus:
+return
+# button == 4 represents scroll up event
+if button == 4.0:
+super().keypress(size, 'up')
+# button == 5 represents scroll down event
+elif button == 5.0:
+super().keypress(size, 'down')
+
 
 class HistoryWindow(urwid.Frame):
 """
-- 
2.17.1

[PATCH v3 04/13] python/aqmp-tui: Add AQMP TUI draft

[G S Niteesh Babu]

Added a draft of AQMP TUI.

Implements the follwing basic features:
1) Command transmission/reception.
2) Shows events asynchronously.
3) Shows server status in the bottom status bar.

Also added necessary pylint, mypy configurations

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/aqmp_tui.py | 333 +++
 python/setup.cfg |  16 +-
 2 files changed, 348 insertions(+), 1 deletion(-)
 create mode 100644 python/qemu/aqmp/aqmp_tui.py

diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
new file mode 100644
index 00..ec9eba0aa7
--- /dev/null
+++ b/python/qemu/aqmp/aqmp_tui.py
@@ -0,0 +1,333 @@
+# Copyright (c) 2021
+#
+# Authors:
+#  Niteesh Babu G S 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import argparse
+import asyncio
+import logging
+from logging import Handler
+import signal
+
+import urwid
+import urwid_readline
+
+from ..qmp import QEMUMonitorProtocol, QMPBadPortError
+from .message import DeserializationError, Message, UnexpectedTypeError
+from .protocol import ConnectError
+from .qmp_client import ExecInterruptedError, QMPClient
+from .util import create_task, pretty_traceback
+
+
+UPDATE_MSG = 'UPDATE_MSG'
+
+# Using root logger to enable all loggers under qemu and asyncio
+LOGGER = logging.getLogger()
+
+
+def format_json(msg):
+"""
+Formats given multiline JSON message into a single line message.
+Converting into single line is more asthetically pleasing when looking
+along with error messages compared to multiline JSON.
+"""
+# FIXME: Use better formatting mechanism. Might break at more complex JSON
+# data.
+msg = msg.replace('\n', '')
+words = msg.split(' ')
+words = [word for word in words if word != '']
+return ' '.join(words)
+
+
+class App(QMPClient):
+def __init__(self, address):
+urwid.register_signal(type(self), UPDATE_MSG)
+self.window = Window(self)
+self.address = address
+self.aloop = None
+super().__init__()
+
+def add_to_history(self, msg):
+urwid.emit_signal(self, UPDATE_MSG, msg)
+
+def _cb_outbound(self, msg):
+# FIXME: I think the ideal way to omit these messages during in-TUI
+# logging will be to add a filter to the logger. We can use regex to
+# filter out messages starting with 'Request:' or 'Response:' but I
+# think a better approach will be encapsulate the message in an object
+# and filter based on the object. Encapsulation of the message will
+# also be necessary when we want different formatting of messages
+# inside TUI.
+handler = LOGGER.handlers[0]
+if not isinstance(handler, TUILogHandler):
+LOGGER.debug('Request: %s', str(msg))
+self.add_to_history('<-- ' + str(msg))
+return msg
+
+def _cb_inbound(self, msg):
+handler = LOGGER.handlers[0]
+if not isinstance(handler, TUILogHandler):
+LOGGER.debug('Response: %s', str(msg))
+self.add_to_history('--> ' + str(msg))
+return msg
+
+async def wait_for_events(self):
+async for event in self.events:
+self.handle_event(event)
+
+async def _send_to_server(self, raw_msg):
+# FIXME: Format the raw_msg in history view to one line. It is not
+# pleasing to see multiple lines JSON object with an error statement.
+try:
+msg = Message(bytes(raw_msg, encoding='utf-8'))
+# Format multiline json into a single line JSON, since it is more
+# pleasing to look along with err message in TUI.
+raw_msg = self.format_json(raw_msg)
+await self._raw(msg, assign_id='id' not in msg)
+except (ValueError, TypeError) as err:
+LOGGER.info('Invalid message: %s', str(err))
+self.add_to_history(f'{raw_msg}: {err}')
+except (DeserializationError, UnexpectedTypeError) as err:
+LOGGER.info('Invalid message: %s', err.error_message)
+self.add_to_history(f'{raw_msg}: {err.error_message}')
+except ExecInterruptedError:
+LOGGER.info('Error server disconnected before reply')
+urwid.emit_signal(self, UPDATE_MSG,
+  '{"error": "Server disconnected before reply"}')
+self._set_status("Server disconnected")
+except Exception as err:
+LOGGER.error('Exception from _send_to_server: %s', str(err))
+raise err
+
+def cb_send_to_server(self, msg):
+create_task(self._send_to_server(msg))
+
+def unhandled_input(self, key):
+if key == 'esc':
+self.kill_app()
+
+def kill_app(self):
+# TODO: Work on the disconnect logic
+create_task(self._kill_app())
+
+async def _kill_app(self):
+# It is ok to call disconnect even in 

[PATCH v3 09/13] python/aqmp-tui: Add QMP connection manager

[G S Niteesh Babu]

Instead of manually connecting and disconnecting from the
server. We now rely on the runstate to manage the QMP
connection.

Along with this the ability to reconnect on certain exceptions
has also been added.

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/aqmp_tui.py | 109 ++-
 1 file changed, 94 insertions(+), 15 deletions(-)

diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
index 0d5ec62cb7..ef91883fa5 100644
--- a/python/qemu/aqmp/aqmp_tui.py
+++ b/python/qemu/aqmp/aqmp_tui.py
@@ -25,8 +25,9 @@
 import urwid_readline
 
 from ..qmp import QEMUMonitorProtocol, QMPBadPortError
+from .error import ProtocolError
 from .message import DeserializationError, Message, UnexpectedTypeError
-from .protocol import ConnectError
+from .protocol import ConnectError, Runstate
 from .qmp_client import ExecInterruptedError, QMPClient
 from .util import create_task, pretty_traceback
 
@@ -67,12 +68,24 @@ def format_json(msg: str) -> str:
 return ' '.join(words)
 
 
+def type_name(mtype: Any) -> str:
+"""
+Returns the type name
+"""
+return type(mtype).__name__
+
+
 class App(QMPClient):
-def __init__(self, address: Union[str, Tuple[str, int]]) -> None:
+def __init__(self, address: Union[str, Tuple[str, int]], num_retries: int,
+ retry_delay: Optional[int]) -> None:
 urwid.register_signal(type(self), UPDATE_MSG)
 self.window = Window(self)
 self.address = address
 self.aloop: Optional[Any] = None  # FIXME: Use more concrete type.
+self.num_retries = num_retries
+self.retry_delay = retry_delay
+self.retry: bool = False
+self.disconnecting: bool = False
 super().__init__()
 
 def add_to_history(self, msg: str, level: Optional[str] = None) -> None:
@@ -119,7 +132,7 @@ def _cb_inbound(self, msg: Message) -> Message:
 LOGGER.info('Error server disconnected before reply')
 urwid.emit_signal(self, UPDATE_MSG,
   '{"error": "Server disconnected before reply"}')
-self._set_status("Server disconnected")
+await self.disconnect()
 except Exception as err:
 LOGGER.error('Exception from _send_to_server: %s', str(err))
 raise err
@@ -136,15 +149,29 @@ def kill_app(self) -> None:
 create_task(self._kill_app())
 
 async def _kill_app(self) -> None:
-# It is ok to call disconnect even in disconnect state
+await self.disconnect()
+LOGGER.debug('Disconnect finished. Exiting app')
+raise urwid.ExitMainLoop()
+
+async def disconnect(self) -> None:
+if self.disconnecting:
+return
 try:
-await self.disconnect()
-LOGGER.debug('Disconnect finished. Exiting app')
+self.disconnecting = True
+await super().disconnect()
+self.retry = True
+except EOFError as err:
+LOGGER.info('disconnect: %s', type_name(err))
+self.retry = True
+except ProtocolError as err:
+LOGGER.info('disconnect: %s', type_name(err))
+self.retry = False
 except Exception as err:
-LOGGER.info('_kill_app: %s', str(err))
-# Let the app crash after providing a proper stack trace
+LOGGER.error('disconnect: Unhandled exception %s', str(err))
+self.retry = False
 raise err
-raise urwid.ExitMainLoop()
+finally:
+self.disconnecting = False
 
 def handle_event(self, event: Message) -> None:
 # FIXME: Consider all states present in qapi/run-state.json
@@ -161,14 +188,61 @@ def _get_formatted_address(self) -> str:
 addr = f'{host}:{port}'
 return addr
 
-async def connect_server(self) -> None:
+async def _retry_connection(self) -> Optional[str]:
+current_retries = 0
+err = None
+# Increase in power sequence of 2 if no delay is provided
+cur_delay = 1
+inc_delay = 2
+if self.retry_delay:
+inc_delay = 1
+cur_delay = self.retry_delay
+# initial try
+await self.connect_server()
+while self.retry and current_retries < self.num_retries:
+LOGGER.info('Connection Failed, retrying in %d', cur_delay)
+status = f'[Retry #{current_retries} ({cur_delay}s)]'
+self._set_status(status)
+
+await asyncio.sleep(cur_delay)
+
+err = await self.connect_server()
+cur_delay *= inc_delay
+# Cap delay to 5mins
+cur_delay = min(cur_delay, 5 * 60)
+current_retries += 1
+# If all retries failed report the last error
+LOGGER.info('All retries failed: %s', str(err))
+return type_name(err)
+
+async def manage_connection(self) -> None:
+while True:
+if 

[PATCH v3 02/13] python: disable pylint errors for aqmp-tui

[G S Niteesh Babu]

Disable missing-docstring and fixme pylint warnings.
This is because since the AQMP is just a prototype
it is currently not documented properly and lot
of todo and fixme's are still in place.

Signed-off-by: G S Niteesh Babu 
---
 python/setup.cfg | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 2573cd7bfb..7a30dd5b09 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -90,6 +90,8 @@ ignore_missing_imports = True
 # --disable=W".
 disable=too-many-function-args,  # mypy handles this with less false positives.
 no-member,  # mypy also handles this better.
+missing-docstring, # FIXME
+fixme, # FIXME
 
 [pylint.basic]
 # Good variable names which should always be accepted, separated by a comma.
-- 
2.17.1

[PATCH v3 06/13] python/aqmp-tui: Added type annotations for aqmp-tui

[G S Niteesh Babu]

This patch adds type annotations for aqmp-tui using
the mypy library.

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/aqmp_tui.py | 79 
 python/setup.cfg |  3 --
 2 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
index ec9eba0aa7..ab9ada793a 100644
--- a/python/qemu/aqmp/aqmp_tui.py
+++ b/python/qemu/aqmp/aqmp_tui.py
@@ -9,8 +9,15 @@
 import argparse
 import asyncio
 import logging
-from logging import Handler
+from logging import Handler, LogRecord
 import signal
+from typing import (
+Any,
+List,
+Optional,
+Tuple,
+Union,
+)
 
 import urwid
 import urwid_readline
@@ -22,13 +29,13 @@
 from .util import create_task, pretty_traceback
 
 
-UPDATE_MSG = 'UPDATE_MSG'
+UPDATE_MSG: str = 'UPDATE_MSG'
 
 # Using root logger to enable all loggers under qemu and asyncio
 LOGGER = logging.getLogger()
 
 
-def format_json(msg):
+def format_json(msg: str) -> str:
 """
 Formats given multiline JSON message into a single line message.
 Converting into single line is more asthetically pleasing when looking
@@ -43,17 +50,17 @@ def format_json(msg):
 
 
 class App(QMPClient):
-def __init__(self, address):
+def __init__(self, address: Union[str, Tuple[str, int]]) -> None:
 urwid.register_signal(type(self), UPDATE_MSG)
 self.window = Window(self)
 self.address = address
-self.aloop = None
+self.aloop: Optional[Any] = None  # FIXME: Use more concrete type.
 super().__init__()
 
-def add_to_history(self, msg):
+def add_to_history(self, msg: str) -> None:
 urwid.emit_signal(self, UPDATE_MSG, msg)
 
-def _cb_outbound(self, msg):
+def _cb_outbound(self, msg: Message) -> Message:
 # FIXME: I think the ideal way to omit these messages during in-TUI
 # logging will be to add a filter to the logger. We can use regex to
 # filter out messages starting with 'Request:' or 'Response:' but I
@@ -67,25 +74,25 @@ def _cb_outbound(self, msg):
 self.add_to_history('<-- ' + str(msg))
 return msg
 
-def _cb_inbound(self, msg):
+def _cb_inbound(self, msg: Message) -> Message:
 handler = LOGGER.handlers[0]
 if not isinstance(handler, TUILogHandler):
 LOGGER.debug('Response: %s', str(msg))
 self.add_to_history('--> ' + str(msg))
 return msg
 
-async def wait_for_events(self):
+async def wait_for_events(self) -> None:
 async for event in self.events:
 self.handle_event(event)
 
-async def _send_to_server(self, raw_msg):
+async def _send_to_server(self, raw_msg: str) -> None:
 # FIXME: Format the raw_msg in history view to one line. It is not
 # pleasing to see multiple lines JSON object with an error statement.
 try:
 msg = Message(bytes(raw_msg, encoding='utf-8'))
 # Format multiline json into a single line JSON, since it is more
 # pleasing to look along with err message in TUI.
-raw_msg = self.format_json(raw_msg)
+raw_msg = format_json(raw_msg)
 await self._raw(msg, assign_id='id' not in msg)
 except (ValueError, TypeError) as err:
 LOGGER.info('Invalid message: %s', str(err))
@@ -102,18 +109,18 @@ def _cb_inbound(self, msg):
 LOGGER.error('Exception from _send_to_server: %s', str(err))
 raise err
 
-def cb_send_to_server(self, msg):
+def cb_send_to_server(self, msg: str) -> None:
 create_task(self._send_to_server(msg))
 
-def unhandled_input(self, key):
+def unhandled_input(self, key: str) -> None:
 if key == 'esc':
 self.kill_app()
 
-def kill_app(self):
+def kill_app(self) -> None:
 # TODO: Work on the disconnect logic
 create_task(self._kill_app())
 
-async def _kill_app(self):
+async def _kill_app(self) -> None:
 # It is ok to call disconnect even in disconnect state
 try:
 await self.disconnect()
@@ -124,7 +131,7 @@ def kill_app(self):
 raise err
 raise urwid.ExitMainLoop()
 
-def handle_event(self, event):
+def handle_event(self, event: Message) -> None:
 # FIXME: Consider all states present in qapi/run-state.json
 if event['event'] == 'SHUTDOWN':
 self._set_status('Server shutdown')
@@ -139,7 +146,7 @@ def _get_formatted_address(self) -> str:
 addr = f'{host}:{port}'
 return addr
 
-async def connect_server(self):
+async def connect_server(self) -> None:
 try:
 await self.connect(self.address)
 addr = self._get_formatted_address()
@@ -148,7 +155,7 @@ def _get_formatted_address(self) -> str:
 LOGGER.info('connect_server: ConnectError %s', str(err))
 self._set_status('Server shutdown')
 
-def 

[PATCH v3 01/13] python/aqmp: Fix wait_closed work-around for python 3.6

[G S Niteesh Babu]

Before this patch the wait_closed work-around for python 3.6
fails during disconnect.
This is a temproray work around for which might be fixed in the
future or will be completely removed when the minimum python
version is raised to 3.7.

This patch was originally written by John Snow 

Signed-off-by: G S Niteesh Babu 
---
 python/qemu/aqmp/util.py | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/python/qemu/aqmp/util.py b/python/qemu/aqmp/util.py
index de0df44cbd..eaa5fc7d5f 100644
--- a/python/qemu/aqmp/util.py
+++ b/python/qemu/aqmp/util.py
@@ -134,7 +134,17 @@ def is_closing(writer: asyncio.StreamWriter) -> bool:
 
 while not transport.is_closing():
 await asyncio.sleep(0)
-await flush(writer)
+
+# This is an ugly workaround, but it's the best I can come up with.
+sock = transport.get_extra_info('socket')
+
+if sock is None:
+# Our transport doesn't have a socket? ...
+# Nothing we can reasonably do.
+return
+
+while sock.fileno() != -1:
+await asyncio.sleep(0)
 
 
 def asyncio_run(coro: Coroutine[Any, Any, T], *, debug: bool = False) -> T:
-- 
2.17.1

[PATCH v3 00/13] AQMP TUI Draft

[G S Niteesh Babu]

Hello all,

Gitlab: https://gitlab.com/niteesh.gs/qemu/-/commits/aqmp-tui-prototype-v3
CI: https://gitlab.com/niteesh.gs/qemu/-/pipelines/345738265

Major revision since V2:
1) Refined QMP connection manager.
2) Added static typing.
3) Allow highlighting specific messages in history box.
4) Allow copying of QMP message from TUI using keyboard shortcut alt-c.

G S Niteesh Babu (13):
  python/aqmp: Fix wait_closed work-around for python 3.6
  python: disable pylint errors for aqmp-tui
  python: Add dependencies for AQMP TUI
  python/aqmp-tui: Add AQMP TUI draft
  python: add entry point for aqmp-tui
  python/aqmp-tui: Added type annotations for aqmp-tui
  python: add optional pygments dependency
  python/aqmp-tui: add syntax highlighting
  python/aqmp-tui: Add QMP connection manager
  python/aqmp-tui: Add scrolling to history box
  python/aqmp-tui: Add ability to highlight messages
  python/aqmp-tui: Add pyperclip dependency
  python/aqmp-tui: Allow copying message from TUI

 python/Pipfile.lock  |  42 +++
 python/qemu/aqmp/aqmp_tui.py | 519 +++
 python/qemu/aqmp/util.py |  12 +-
 python/setup.cfg |  34 ++-
 4 files changed, 605 insertions(+), 2 deletions(-)
 create mode 100644 python/qemu/aqmp/aqmp_tui.py

-- 
2.17.1

Re: [PATCH for-6.2 45/53] target/arm: Implement MVE FP max/min across vector

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:
  
  {

+  VMAXNMAV   1110 1110 1110  11 00    0 0 . 0 ... 0 @vmaxnmv size=2
+  VMINNMAV   1110 1110 1110  11 00    1 0 . 0 ... 0 @vmaxnmv size=2
+  VMAXNMV1110 1110 1110  11 10    0 0 . 0 ... 0 @vmaxnmv size=2
+  VMINNMV1110 1110 1110  11 10    1 0 . 0 ... 0 @vmaxnmv size=2
VMAXV_S1110 1110 1110  .. 10    0 0 . 0 ... 0 @vmaxv
VMINV_S1110 1110 1110  .. 10    1 0 . 0 ... 0 @vmaxv
VMAXAV 1110 1110 1110  .. 00    0 0 . 0 ... 0 @vmaxv


Looks like we could usefully have two [] blocks in here, for these 4 insns, and the 
previous 4 minmax.




+#define DO_FP_VMAXMINV(OP, ESIZE, TYPE, FTYPE, ABS, FN) \


Drop TYPE and just use FTYPE.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH for-6.2 44/53] softfloat: Remove assertion preventing silencing of NaN in default-NaN mode

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

In commit a777d6033447a we added an assertion to parts_silence_nan() that
prohibits calling float*_silence_nan() when in default-NaN mode.
This ties together a property of the output ("do we generate a default
NaN when the result is a NaN?") with an operation on an input ("silence
this input NaN").

It's true that most of the time when in default-NaN mode you won't
need to silence an input NaN, because you can just produce the
default NaN as the result instead.  But some functions like
float*_maxnum() are defined to be able to work with quiet NaNs, so
silencing an input SNaN is still reasonable.  In particular, the
upcoming implementation of MVE VMAXNMV would fall over this assertion
if we didn't delete it.

Delete the assertion.

Signed-off-by: Peter Maydell
---
  fpu/softfloat-specialize.c.inc | 1 -
  1 file changed, 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 43/53] target/arm: Implement MVE fp-with-scalar VFMA, VFMAS

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE fp-with-scalar VFMA and VFMAS insns.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  6 ++
  target/arm/mve.decode  | 14 +++---
  target/arm/mve_helper.c| 37 +
  target/arm/translate-mve.c |  2 ++
  4 files changed, 56 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 42/53] target/arm: Implement MVE scalar fp insns

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE scalar floating point insns VADD, VSUB and VMUL.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  9 +
  target/arm/mve.decode  | 27 +--
  target/arm/mve_helper.c| 34 ++
  target/arm/translate-mve.c | 20 
  4 files changed, 84 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 41/53] target/arm: Implement MVE VMAXNMA and VMINNMA

[Richard Henderson]

On 7/29/21 1:15 AM, Peter Maydell wrote:

Implement the MVE VMAXNMA and VMINNMA insns; these are 2-operand, but
the destination register must be the same as one of the source
registers.

We defer the decode of the size in bit 28 to the individual insn
patterns rather than doing it in the format, because otherwise we
would have a single insn pattern that overlapped with two groups (eg
VMAXNMA with the VMULH_S and VMULH_U groups). Having two insn
patterns per insn seems clearer than a complex multilevel nesting
of overlapping and non-overlapping groups.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  6 ++
  target/arm/mve.decode  | 11 +++
  target/arm/mve_helper.c| 25 +
  target/arm/translate-mve.c |  2 ++
  4 files changed, 44 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 40/53] target/arm: Implement MVE VCMUL and VCMLA

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

+#define DO_VCMULH(N, M, D, S) float16_mul(N, M, S)
+#define DO_VCMULS(N, M, D, S) float32_mul(N, M, S)
+
+#define DO_VCMLAH(N, M, D, S) float16_muladd(N, M, D, 0, S)
+#define DO_VCMLAS(N, M, D, S) float32_muladd(N, M, D, 0, S)
+
+DO_VCMLA(vcmul0h, 2, uint16_t, 0, float16_chs, DO_VCMULH)
+DO_VCMLA(vcmul0s, 4, uint32_t, 0, float32_chs, DO_VCMULS)
+DO_VCMLA(vcmul90h, 2, uint16_t, 1, float16_chs, DO_VCMULH)
+DO_VCMLA(vcmul90s, 4, uint32_t, 1, float32_chs, DO_VCMULS)
+DO_VCMLA(vcmul180h, 2, uint16_t, 2, float16_chs, DO_VCMULH)
+DO_VCMLA(vcmul180s, 4, uint32_t, 2, float32_chs, DO_VCMULS)
+DO_VCMLA(vcmul270h, 2, uint16_t, 3, float16_chs, DO_VCMULH)
+DO_VCMLA(vcmul270s, 4, uint32_t, 3, float32_chs, DO_VCMULS)


Here you don't need to pass in floatN_chs if you use floatN as TYPE.  The macros to merge 
mul and muladd are clever.


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 39/53] target/arm: Implement MVE VFMA and VFMS

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

+r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], d[H##ESIZE(e)],  \
+   0, fpst);\
+mergemask([H##ESIZE(e)], r, mask);\
+}   \
+mve_advance_vpt(env);   \
+}
+
+#define DO_VFMS16(N, M, D, F, S) float16_muladd(float16_chs(N), M, D, F, S)
+#define DO_VFMS32(N, M, D, F, S) float32_muladd(float32_chs(N), M, D, F, S)
+
+DO_VFMA(vfmah, 2, uint16_t, float16_muladd)
+DO_VFMA(vfmas, 4, uint32_t, float32_muladd)
+DO_VFMA(vfmsh, 2, uint16_t, DO_VFMS16)
+DO_VFMA(vfmss, 4, uint32_t, DO_VFMS32)


Here's where I think passing float16/float32 as the type will pay off, with

  r = n[H##SIZE(e)];
  if (CHS) {
r = TYPE##_chs(r);
  }
  r = TYPE##_muladd(r, m[...], d[...], 0, fpst);


r~

Re: [PATCH for-6.2 36/53] target/arm: Implement MVE VADD (floating-point)

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

+DO_2OP_FP(vfaddh, 2, uint16_t, float16_add)
+DO_2OP_FP(vfadds, 4, uint32_t, float32_add)


Use float16 and float32 types here?
It'll be more interesting with some of the other macros later...


r~

RE: [PATCH for-6.2 5/8] arch_init.h: Add QEMU_ARCH_HEXAGON

[Taylor Simpson]

> -Original Message-
> From: Qemu-devel  bounces+tsimpson=quicinc@nongnu.org> On Behalf Of Peter Maydell
> Sent: Friday, July 30, 2021 5:00 AM
> To: qemu-devel@nongnu.org
> Cc: Paolo Bonzini ; Markus Armbruster
> ; Eduardo Habkost 
> Subject: [PATCH for-6.2 5/8] arch_init.h: Add QEMU_ARCH_HEXAGON
> 
> When Hexagon was added we forgot to add it to the QEMU_ARCH_*
> enumeration.  This doesn't cause a visible effect because at the moment
> Hexagon is linux-user only and the QEMU_ARCH_* constants are only used
> in softmmu, but we might as well add it in, since it's the only architecture
> currently missing from the list.
> 
> Signed-off-by: Peter Maydell 


> +QEMU_ARCH_HEXAGON = (1 << 22),

Reviewed-by: Taylor Simpson 

Re: [PATCH for-6.2 39/53] target/arm: Implement MVE VFMA and VFMS

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

Implement the MVE VFMA and VFMS insns.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  6 ++
  target/arm/mve.decode  |  3 +++
  target/arm/mve_helper.c| 36 
  target/arm/translate-mve.c |  2 ++
  4 files changed, 47 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 38/53] target/arm: Implement MVE VCADD

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

Implement the MVE VCADD insn.  Note that here the size bit is the
opposite sense to the other 2-operand fp insns.

We don't check for the sz == 1 && Qd == Qm UNPREDICTABLE case,
because that would mean we can't use the DO_2OP_FP macro in
translate-mve.c.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h|  6 ++
  target/arm/mve.decode  |  8 
  target/arm/mve_helper.c| 40 ++
  target/arm/translate-mve.c |  4 +++-
  4 files changed, 57 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 37/53] target/arm: Implement MVE VSUB, VMUL, VABD, VMAXNM, VMINNM

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

Implement more simple 2-operand floating point MVE insns.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h| 15 +++
  target/arm/mve.decode  |  6 ++
  target/arm/mve_helper.c| 24 
  target/arm/translate-mve.c |  5 +
  4 files changed, 50 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 36/53] target/arm: Implement MVE VADD (floating-point)

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

Implement the MVE VADD (floating-point) insn.  Handling of this is
similar to the 2-operand integer insns, except that we must take care
to only update the floating point exception status if the least
significant bit of the predicate mask for each element is active.

Signed-off-by: Peter Maydell
---
  target/arm/helper-mve.h |  3 +++
  target/arm/translate.h  |  6 ++
  target/arm/mve.decode   | 10 ++
  target/arm/mve_helper.c | 37 +
  target/arm/translate-mve.c  | 17 +
  target/arm/translate-neon.c |  6 --
  6 files changed, 73 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 26/53] target/arm: Implement MVE VMLA

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

Implement the MVE VMLA insn, which multiplies a vector by a scalar
and accumulates into another vector.

Signed-off-by: Peter Maydell
---
Changes v1->v2: don't decode U bit
---
  target/arm/helper-mve.h| 4 
  target/arm/mve.decode  | 1 +
  target/arm/mve_helper.c| 5 +
  target/arm/translate-mve.c | 1 +
  4 files changed, 11 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 21/53] target/arm: Implement MVE integer min/max across vector

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

Implement the MVE integer min/max across vector insns
VMAXV, VMINV, VMAXAV and VMINAV, which find the maximum
from the vector elements and a general purpose register,
and store the maximum back into the general purpose
register.

These insns overlap with VRMLALDAVH (they use what would
be RdaHi=0b110).

Signed-off-by: Peter Maydell
---
Changes v1->v2: Drop the harmless but unnecessary
"take abs value of 'n'" part of do_maxa() and do_mina()
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 10/53] target/arm: Fix VPT advance when ECI is non-zero

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

We were not paying attention to the ECI state when advancing the VPT
state.  Architecturally, VPT state advance happens for every beat
(see the pseudocode VPTAdvance()), so on every beat the 4 bits of
VPR.P0 corresponding to the current beat are inverted if required,
and at the end of beats 1 and 3 the VPR MASK fields are updated.
This means that if the ECI state says we should not be executing all
4 beats then we need to skip some of the updating of the VPR that we
currently do in mve_advance_vpt().

Signed-off-by: Peter Maydell
---
  target/arm/mve_helper.c | 24 +---
  1 file changed, 17 insertions(+), 7 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 07/53] target/arm: Fix MVE 48-bit SQRSHRL for small right shifts

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

We got an edge case wrong in the 48-bit SQRSHRL implementation: if
the shift is to the right, although it always makes the result
smaller than the input value it might not be within the 48-bit range
the result is supposed to be if the input had some bits in [63..48]
set and the shift didn't bring all of those within the [47..0] range.

Handle this similarly to the way we already do for this case in
do_uqrshl48_d(): extend the calculated result from 48 bits,
and return that if not saturating or if it doesn't change the
result; otherwise fall through to return a saturated value.

Signed-off-by: Peter Maydell 
---
Not squashed into the previous patch because that one has already
been reviewed, so as this fixes a different edge case I thought
it clearer kept separate.
---


Reviewed-by: Richard Henderson 


  target/arm/mve_helper.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/arm/mve_helper.c b/target/arm/mve_helper.c
index 5730b48f35e..1a4b2ef8075 100644
--- a/target/arm/mve_helper.c
+++ b/target/arm/mve_helper.c
@@ -1563,6 +1563,8 @@ uint64_t HELPER(mve_uqrshll)(CPUARMState *env, uint64_t 
n, uint32_t shift)
  static inline int64_t do_sqrshl48_d(int64_t src, int64_t shift,
  bool round, uint32_t *sat)
  {
+int64_t val, extval;
+
  if (shift <= -48) {
  /* Rounding the sign bit always produces 0. */
  if (round) {
@@ -1572,9 +1574,14 @@ static inline int64_t do_sqrshl48_d(int64_t src, int64_t 
shift,
  } else if (shift < 0) {
  if (round) {
  src >>= -shift - 1;
-return (src >> 1) + (src & 1);
+val = (src >> 1) + (src & 1);
+} else {
+val = src >> -shift;
+}
+extval = sextract64(val, 0, 48);
+if (!sat || val == extval) {
+return extval;
  }
-return src >> -shift;


I'll note two things:

(1) The val == extval check could be sunk to the end of the function and shared with the 
left shift,


(2) sat will never be unset, as #48 is encoded as sat=1 in the insn.


r~

Re: [PATCH for-6.2 03/53] target/arm: Fix MVE VSLI by 0 and VSRI by

[Richard Henderson]

On 7/29/21 1:14 AM, Peter Maydell wrote:

In the MVE shift-and-insert insns, we special case VSLI by 0
and VSRI by . VSRI by  means "don't update the destination",
which is what we've implemented. However VSLI by 0 is "set
destination to the input", so we don't want to use the same
special-casing that we do for VSRI by .

Since the generic logic gives the right answer for a shift
by 0, just use that.

Signed-off-by: Peter Maydell
---
  target/arm/mve_helper.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 4/8] meson.build: Define QEMU_ARCH in config-target.h

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

Instead of using an ifdef ladder in arch_init.c (which we then have
to manually update every time we add or remove a target
architecture), have meson.build put "#define QEMU_ARCH QEMU_ARCH_FOO"
in the config-target.h file.

Signed-off-by: Peter Maydell
---
  meson.build |  2 ++
  softmmu/arch_init.c | 41 -
  2 files changed, 2 insertions(+), 41 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 6/8] arch_init.h: Move QEMU_ARCH_VIRTIO_* to qdev-monitor.c

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

The QEMU_ARCH_VIRTIO_* defines are used only in one file,
qdev-monitor.c. Move them to that file.

Signed-off-by: Peter Maydell
---
  include/sysemu/arch_init.h | 9 -
  softmmu/qdev-monitor.c | 9 +
  2 files changed, 9 insertions(+), 9 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 8/8] stubs: Remove unused arch_type.c stub

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

We added a stub for the arch_type global in commit 5964ed56d9a1 so
that we could compile blockdev.c into the tools.  However, in commit
9db1d3a2be9bf we removed the only use of arch_type from blockdev.c.
The stub is therefore no longer needed, and we can delete it again,
together with the QEMU_ARCH_NONE value that only the stub was using.

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 3/8] softmmu/arch_init.c: Trim down include list

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

arch_init.c does very little but has a long list of #include lines.
Remove all the unnecessary ones.

Signed-off-by: Peter Maydell
---
  softmmu/arch_init.c | 7 ---
  1 file changed, 7 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 5/8] arch_init.h: Add QEMU_ARCH_HEXAGON

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

When Hexagon was added we forgot to add it to the QEMU_ARCH_*
enumeration.  This doesn't cause a visible effect because at the
moment Hexagon is linux-user only and the QEMU_ARCH_* constants are
only used in softmmu, but we might as well add it in, since it's the
only architecture currently missing from the list.

Signed-off-by: Peter Maydell
---
  include/sysemu/arch_init.h | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 7/8] arch_init.h: Don't include arch_init.h unnecessarily

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

arch_init.h only defines the QEMU_ARCH_* enumeration and the
arch_type global. Don't include it in files that don't use those.

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 2/8] monitor: Use accel_find("kvm") instead of kvm_available()

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

The kvm_available() function reports whether KVM support was
compiled into the QEMU binary; it returns the value of the
CONFIG_KVM define.

The only place in the codebase where we use this function is
in qmp_query_kvm(). Now that accelerators are based on QOM
classes we can instead use accel_find("kvm") and remove the
kvm_available() function.

Signed-off-by: Peter Maydell
---
  include/sysemu/arch_init.h | 2 --
  monitor/qmp-cmds.c | 2 +-
  softmmu/arch_init.c| 9 -
  3 files changed, 1 insertion(+), 12 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-6.2 1/8] softmmu: Use accel_find("xen") instead of xen_available()

[Richard Henderson]

On 7/30/21 12:59 AM, Peter Maydell wrote:

The xen_available() function is used only to produce an error
for some Xen-specific command line options in QEMU binaries where
Xen support was not compiled in: it just returns the value of
the CONFIG_XEN define.

Now that accelerators are QOM classes, we can check for
"does this binary have Xen compiled in" with accel_find("xen"),
and drop the xen_available() function.

Signed-off-by: Peter Maydell
---
  include/sysemu/arch_init.h | 1 -
  softmmu/arch_init.c| 9 -
  softmmu/vl.c   | 6 +++---
  3 files changed, 3 insertions(+), 13 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PULL 2/2] target/nios2: Mark raise_exception() as noreturn

[Richard Henderson]

From: Philippe Mathieu-Daudé 

Raised exceptions don't return, so mark the helper with noreturn.

Fixes: 032c76bc6f9 ("nios2: Add architecture emulation support")
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20210729101315.2318714-1-f4...@amsat.org>
Signed-off-by: Richard Henderson 
---
 target/nios2/helper.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/nios2/helper.h b/target/nios2/helper.h
index b0cb9146a5..6c8f0b5b35 100644
--- a/target/nios2/helper.h
+++ b/target/nios2/helper.h
@@ -18,7 +18,7 @@
  * 
  */
 
-DEF_HELPER_2(raise_exception, void, env, i32)
+DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
 
 #if !defined(CONFIG_USER_ONLY)
 DEF_HELPER_2(mmu_read_debug, void, env, i32)
-- 
2.25.1

[PULL 1/2] accel/tcg: Remove double bswap for helper_atomic_sto_*_mmu

[Richard Henderson]

This crept in as either a cut-and-paste error, or rebase error.

Fixes: cfec388518d ("atomic_template: add inline trace/plugin helpers")
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210729004647.282017-24-richard.hender...@linaro.org>
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index d89af4cc1e..8098a1be31 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -251,7 +251,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
  PAGE_WRITE, retaddr);
 uint16_t info = atomic_trace_st_pre(env, addr, oi);
 
-val = BSWAP(val);
 val = BSWAP(val);
 atomic16_set(haddr, val);
 ATOMIC_MMU_CLEANUP;
-- 
2.25.1

[PULL 0/2] tcg patch queue for rc2

[Richard Henderson]

The following changes since commit dbdc621be937d9efe3e4dff994e54e8eea051f7a:

  Merge remote-tracking branch 
'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2021-07-30 
09:14:56 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 236f6709ae0da224314c3344c339ed0dc07c15cf:

  target/nios2: Mark raise_exception() as noreturn (2021-07-30 08:23:12 -1000)


Fix double bswap in 16-byte atomic store
Mark nios2 raise_exception noreturn


Philippe Mathieu-Daudé (1):
  target/nios2: Mark raise_exception() as noreturn

Richard Henderson (1):
  accel/tcg: Remove double bswap for helper_atomic_sto_*_mmu

 accel/tcg/atomic_template.h | 1 -
 target/nios2/helper.h   | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Misc questions

[Kenneth Adam Miller]

Hello,


I think I need a concurrent radix tree that is written to work with atomic
updates. I would like to ask if anyone knows of one within qemu? Or at
least to efficiently obtain the page address/mmu index corresponding for a
given address.

Is there any documentation on cpu_mmu_index? Each target provides its own
implementation, but I find it hard to gather what is expected of this
except to read all of the references in the periphery.

Lastly, ok I have a working build going and it runs without crashing. But
when I pass it my image, all it does is show the qemu shell prompt,
"(qemu)". I'm not sure why it doesn't start running, I think it should at
least encounter new opcodes and then stop. Can anybody think of anything
that I might be missing?

Re: [PATCH] MAINTAINERS: Added myself as a reviewer for acpi/smbios subsystem

[Philippe Mathieu-Daudé]

On 7/30/21 7:55 PM, Ani Sinha wrote:
> I have developed an interest in this space and hopefully can lend some
> helping hand to Igor and Michael in reviewing simpler patches.

Help is welcome IMHO, so:
Reviewed-by: Philippe Mathieu-Daudé 

> Signed-off-by: Ani Sinha 
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4256ad1adb..1c90ea4e6b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1739,6 +1739,7 @@ F: docs/specs/*pci*
>  ACPI/SMBIOS
>  M: Michael S. Tsirkin 
>  M: Igor Mammedov 
> +R: Ani Sinha 
>  S: Supported
>  F: include/hw/acpi/*
>  F: include/hw/firmware/smbios.h
> 

Re: [PATCH v3 0/2] x86/sev: Measured Linux SEV guest with kernel/initrd/cmdline

[Connor Kuehl]

On Fri Jul 30, 2021 at 1:02 PM CDT, Dov Murik wrote:
>
>
> > Awesome! Unfortunately, it's looking like we'll have to wait[1] for QEMU to
> > thaw before this series goes in.
> > 
>
> Thanks for explaining this. Do I need to do anything after 6.1 is
> released? Ping? Rebase and re-send?

Rebase and re-send. I think your patches already have the Reviewed-by
tags in the patch descriptions, but if that's not the case, make sure
you add them for the re-send so it's obvious that the patches have
already been reviewed.

Thank you,

Connor

Re: QEMU on x64

[Peter Maydell]

On Fri, 30 Jul 2021 at 19:05, Christopher Caulfield
 wrote:
> This is Christopher from the debugging experiences team at Microsoft focused 
> on kernel debugging. I am reaching out with a few questions about QEMU on x64.
>
> Is it possible for the QEMU-x86-64 GDB Server to send the full set of x64 
> system registers (whether they are included in a separated system xml file or 
> as part of the core registers xml file)?

Do you mean "is it possible for somebody to write code for
QEMU to make it do that", or "does QEMU do it today if you pass
it the right command line option" ? The answer to the former
is "yes", to the latter "no". (If you want the debugger to
be able to write to the system registers this might be a little
trickier, mostly in terms of "auditing the code to make sure this
can't confuse QEMU if you change some sysreg under its feet.".)

> e.g. System registers missing from i386-64bit.xml file

> DWORD64 IDTBase;
> DWORD64 IDTLimit;
> DWORD64 GDTBase;
> DWORD64 GDTLimit;
> DWORD SelLDT;
> SEG64_DESC_INFO SegLDT;
> DWORD SelTSS;
> SEG64_DESC_INFO SegTSS;
>
> How can I access x64 MSR registers by using the QEMU-x86-64 GDB server?
>
> #define MSR_EFER 0xc080 // extended function enable register

EFER is in the xml ("x64_efer") so should be already accessible.
For anything else you're going to need to write some code to
make it happen.

>is there any plan to support reading/writing to MSRs via QEMU-x86-64 GDB 
>server?

Not that I know of. We'd be happy to review patches if you want to
write them.

thanks
-- PMM

QEMU on x64

[Christopher Caulfield]

Hi QEMU community,

This is Christopher from the debugging experiences team at Microsoft
focused on kernel debugging. I am reaching out with a few questions about
QEMU on x64.


   1. Is it possible for the QEMU-x86-64 GDB Server to send the full set of
   x64 system registers (whether they are included in a separated system xml
   file or as part of the core registers xml file)?
  - e.g. System registers missing from i386-64bit.xml file

  DWORD64 IDTBase;
  DWORD64 IDTLimit;
  DWORD64 GDTBase;
  DWORD64 GDTLimit;
  DWORD SelLDT;
  SEG64_DESC_INFO SegLDT;
  DWORD SelTSS;
  SEG64_DESC_INFO SegTSS;
  2. How can I access x64 MSR registers by using the QEMU-x86-64 GDB
   server?
  - #define MSR_EFER 0xc080 // extended function enable register
  #define MSR_STAR 0xc081 // system call selectors
  #define MSR_LSTAR 0xc082 // system call 64-bit entry
  #define MSR_CSTAR 0xc083 // system call 32-bit entry
   3. Going off of #2 - can you access it via reading GDB memory command?
   if not - is there any plan to support reading/writing to MSRs via
   QEMU-x86-64 GDB server?


Thank you for taking time to answer our questions! :)

-Christopher
LinkedIn  | Twitter

Re: [PATCH v3 0/2] x86/sev: Measured Linux SEV guest with kernel/initrd/cmdline

[Dov Murik]

On 30/07/2021 17:47, Connor Kuehl wrote:
> On Thu Jul 29, 2021 at 2:31 PM CDT, Dov Murik wrote:
>> The OVMF companion series has been reviewed by the new OVMF maintainer
>> and merged to edk2 master branch as of edk2 commit 514b3aa08ece [1].
>>
>> [1] https://github.com/tianocore/edk2/commit/514b3aa08ece
> 
> Awesome! Unfortunately, it's looking like we'll have to wait[1] for QEMU to
> thaw before this series goes in.
> 

Thanks for explaining this.  Do I need to do anything after 6.1 is
released? Ping? Rebase and re-send?

-Dov


> Connor
> 
> [1] https://wiki.qemu.org/Planning/6.1
> 

[PATCH] MAINTAINERS: Added myself as a reviewer for acpi/smbios subsystem

[Ani Sinha]

I have developed an interest in this space and hopefully can lend some
helping hand to Igor and Michael in reviewing simpler patches.

Signed-off-by: Ani Sinha 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4256ad1adb..1c90ea4e6b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1739,6 +1739,7 @@ F: docs/specs/*pci*
 ACPI/SMBIOS
 M: Michael S. Tsirkin 
 M: Igor Mammedov 
+R: Ani Sinha 
 S: Supported
 F: include/hw/acpi/*
 F: include/hw/firmware/smbios.h
-- 
2.25.1

Re: [PATCH for-6.2 07/43] target/ppc: Set fault address in ppc_cpu_do_unaligned_access

[Cédric Le Goater]

On 7/30/21 7:13 PM, Cédric Le Goater wrote:
> On 7/29/21 8:05 PM, Richard Henderson wrote:
>> On 7/29/21 3:44 AM, Peter Maydell wrote:
>>> On Thu, 29 Jul 2021 at 01:51, Richard Henderson
>>>  wrote:

 We ought to have been recording the virtual address for reporting
 to the guest trap handler.

 Cc: qemu-...@nongnu.org
 Signed-off-by: Richard Henderson 
 ---
   target/ppc/excp_helper.c | 2 ++
   1 file changed, 2 insertions(+)

 diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
 index a79a0ed465..0b2c6de442 100644
 --- a/target/ppc/excp_helper.c
 +++ b/target/ppc/excp_helper.c
 @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr 
 vaddr,
   CPUPPCState *env = cs->env_ptr;
   uint32_t insn;

 +    env->spr[SPR_DAR] = vaddr;
 +
>>>
>>> Is this the right SPR for all PPC variants? For instance the
>>> kernel's code in arch/powerpc/kernel/exceptions-64e.S looks
>>> in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR.
> 
> Indeed :/
> 
>> I have no idea.  I glanced through a handful of the mmu's, and looked at the 
>> current BookS docs, but that's certainly not all.
> 
> I took a look at some more and for instance, e300 uses DAR and e500, 405, 476 
> use DEAR. 
> 
> DAR should be consistent over the server processors.


and  is_book3s_arch2x(env) is a good way to test.

Thanks,

C. 

Re: [PATCH for-6.2 07/43] target/ppc: Set fault address in ppc_cpu_do_unaligned_access

[Cédric Le Goater]

On 7/29/21 8:05 PM, Richard Henderson wrote:
> On 7/29/21 3:44 AM, Peter Maydell wrote:
>> On Thu, 29 Jul 2021 at 01:51, Richard Henderson
>>  wrote:
>>>
>>> We ought to have been recording the virtual address for reporting
>>> to the guest trap handler.
>>>
>>> Cc: qemu-...@nongnu.org
>>> Signed-off-by: Richard Henderson 
>>> ---
>>>   target/ppc/excp_helper.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>> index a79a0ed465..0b2c6de442 100644
>>> --- a/target/ppc/excp_helper.c
>>> +++ b/target/ppc/excp_helper.c
>>> @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr 
>>> vaddr,
>>>   CPUPPCState *env = cs->env_ptr;
>>>   uint32_t insn;
>>>
>>> +    env->spr[SPR_DAR] = vaddr;
>>> +
>>
>> Is this the right SPR for all PPC variants? For instance the
>> kernel's code in arch/powerpc/kernel/exceptions-64e.S looks
>> in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR.

Indeed :/

> I have no idea.  I glanced through a handful of the mmu's, and looked at the 
> current BookS docs, but that's certainly not all.

I took a look at some more and for instance, e300 uses DAR and e500, 405, 476 
use DEAR. 

DAR should be consistent over the server processors.

C.

> 
> I'll note that if we do need to set different regs for different mmus, we'll 
> probably want to standardize on this one for user-only, like we did for the 
> user-only copy of ppc_cpu_tlb_fill.
> 
> 
> r~
> 

[PATCH 0/2] cocoa.m: keyboard quality of life reborn

[John Arbuckle]

These patches can really help improve keyboard usage with a guest.
Based on patches by Gustavo Noronha Silva . 

John Arbuckle (2):
  Add full keyboard grab support
  Add ability to swap command/meta and options keys

 ui/cocoa.m | 178 ++---
 1 file changed, 170 insertions(+), 8 deletions(-)

-- 
2.24.3 (Apple Git-128)

[PATCH 1/2] ui/cocoa.m: Add full keyboard grab support

[John Arbuckle]

There are keyboard shortcuts that are vital for use in a guest that runs Mac OS.
These shortcuts are reserved for Mac OS use only which makes having the guest
see them impossible on a Mac OS host - until now. This patch will enable the
user to decide if the guest should see all keyboard shortcuts using a menu item.
This patch adds a new menu called Options and a new menu item called
"Full Keyboard Grab". Simply selecting this menu item will turn the feature on
or off at any time. Mac OS requires the user to enable access to assistive
devices to use this feature. How to do this varies with each Mac OS version.
Based on patch by Gustavo Noronha Silva . 

Signed-off-by: John Arbuckle 
---
 ui/cocoa.m | 112 +
 1 file changed, 112 insertions(+)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 9f72844b07..fdef9e9901 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -114,6 +114,9 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 typedef void (^CodeBlock)(void);
 typedef bool (^BoolCodeBlock)(void);
 
+static CFMachPortRef eventsTap = NULL;
+static CFRunLoopSourceRef eventsTapSource = NULL;
+
 static void with_iothread_lock(CodeBlock block)
 {
 bool locked = qemu_mutex_iothread_locked();
@@ -332,10 +335,27 @@ - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
 - (void) raiseAllKeys;
+- (void) setFullGrab;
 @end
 
 QemuCocoaView *cocoaView;
 
+// Part of the full keyboard grab system
+static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type,
+CGEventRef cgEvent, void *userInfo)
+{
+QemuCocoaView *cocoaView = (QemuCocoaView*) userInfo;
+NSEvent* event = [NSEvent eventWithCGEvent:cgEvent];
+if ([cocoaView isMouseGrabbed] && [cocoaView handleEvent:event]) {
+COCOA_DEBUG("Global events tap: qemu handled the event, capturing!\n");
+return NULL;
+}
+COCOA_DEBUG("Global events tap: qemu did not handle the event, letting it"
+" through...\n");
+
+return cgEvent;
+}
+
 @implementation QemuCocoaView
 - (id)initWithFrame:(NSRect)frameRect
 {
@@ -361,6 +381,12 @@ - (void) dealloc
 }
 
 qkbd_state_free(kbd);
+if (eventsTap) {
+CFRelease(eventsTap);
+}
+if (eventsTapSource) {
+CFRelease(eventsTapSource);
+}
 [super dealloc];
 }
 
@@ -1086,6 +1112,50 @@ - (void) raiseAllKeys
 qkbd_state_lift_all_keys(kbd);
 });
 }
+
+// Inserts the event tap.
+// This enables us to receive keyboard events that Mac OS would
+// otherwise not let us see - like Command-Option-Esc.
+- (void) setFullGrab
+{
+COCOA_DEBUG("QemuCocoaView: setFullGrab\n");
+NSString *advice = @"Try enabling access to assistive devices";
+CGEventMask mask = CGEventMaskBit(kCGEventKeyDown) |
+CGEventMaskBit(kCGEventKeyUp) | CGEventMaskBit(kCGEventFlagsChanged);
+eventsTap = CGEventTapCreate(kCGHIDEventTap, kCGHeadInsertEventTap,
+ kCGEventTapOptionDefault, mask, 
handleTapEvent,
+ self);
+if (!eventsTap) {
+@throw [NSException
+ exceptionWithName:@"Tap failure"
+reason:[NSString stringWithFormat:@"%@\n%@", @"Could not "
+"create event tap.", advice]
+userInfo:nil];
+} else {
+COCOA_DEBUG("Global events tap created! Will capture system key"
+" combos.\n");
+}
+
+eventsTapSource =
+CFMachPortCreateRunLoopSource(kCFAllocatorDefault, eventsTap, 0);
+if (!eventsTapSource ) {
+@throw [NSException
+ exceptionWithName:@"Tap failure"
+ reason:@"Could not obtain current CFRunLoop source."
+userInfo:nil];
+}
+CFRunLoopRef runLoop = CFRunLoopGetCurrent();
+if (!runLoop) {
+   @throw [NSException
+ exceptionWithName:@"Tap failure"
+ reason:@"Could not obtain current CFRunLoop."
+userInfo:nil];
+}
+
+CFRunLoopAddSource(runLoop, eventsTapSource, kCFRunLoopDefaultMode);
+CFRelease(eventsTapSource);
+}
+
 @end
 
 
@@ -1117,6 +1187,7 @@ - (void)openDocumentation:(NSString *)filename;
 - (IBAction) do_about_menu_item: (id) sender;
 - (void)make_about_window;
 - (void)adjustSpeed:(id)sender;
+- (IBAction)doFullGrab:(id)sender;
 @end
 
 @implementation QemuCocoaAppController
@@ -1569,6 +1640,35 @@ - (void)adjustSpeed:(id)sender
 COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), 
'%');
 }
 
+// The action method to the 'Options->Full Keyboard Grab' menu item
+- (IBAction)doFullGrab:(id) sender
+{
+@try
+{
+// Set the state of the menu item
+// if already checked
+if ([sender state] == NSControlStateValueOn) {
+// remove runloop source
+CFRunLoopSourceInvalidate(eventsTapSource);
+if (!eventsTap) {
+CFRelease(eventsTap);
+}
+[sender 

[PATCH 2/2] ui/cocoa.m: Add ability to swap command/meta and options keys

[John Arbuckle]

For users who are use to using a Macintosh keyboard having to use a PC keyboard
can be a pain because the Command/meta and Option/Alt keys are switched. To
make life easier this option is added to allow the user to switch how the guest
reacts to each of these keys being pushed. It can make a Macintosh keyboard user
feel almost at home with a PC keyboard. The same could be said for PC keyboard
users who have to use a Macinosh keyboard.
Based on patch by Gustavo Noronha Silva .

Signed-off-by: John Arbuckle 
---
 ui/cocoa.m | 66 +++---
 1 file changed, 58 insertions(+), 8 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index fdef9e9901..98596d5f38 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -116,6 +116,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 
 static CFMachPortRef eventsTap = NULL;
 static CFRunLoopSourceRef eventsTapSource = NULL;
+static bool swap_command_option_keys = false;
 
 static void with_iothread_lock(CodeBlock block)
 {
@@ -810,12 +811,22 @@ - (bool) handleEventLocked:(NSEvent *)event
 qkbd_state_key_event(kbd, Q_KEY_CODE_CTRL_R, false);
 }
 if (!(modifiers & NSEventModifierFlagOption)) {
-qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
-qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
+if (swap_command_option_keys) {
+qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
+qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
+} else {
+qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
+qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
+}
 }
 if (!(modifiers & NSEventModifierFlagCommand)) {
-qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
-qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
+if (swap_command_option_keys) {
+qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
+qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
+} else {
+qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
+qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
+}
 }
 
 switch ([event type]) {
@@ -847,13 +858,22 @@ - (bool) handleEventLocked:(NSEvent *)event
 
 case kVK_Option:
 if (!!(modifiers & NSEventModifierFlagOption)) {
-[self toggleKey:Q_KEY_CODE_ALT];
+if (swap_command_option_keys) {
+[self toggleKey:Q_KEY_CODE_META_L];
+} else {
+[self toggleKey:Q_KEY_CODE_ALT];
+}
+
 }
 break;
 
 case kVK_RightOption:
 if (!!(modifiers & NSEventModifierFlagOption)) {
-[self toggleKey:Q_KEY_CODE_ALT_R];
+if (swap_command_option_keys) {
+[self toggleKey:Q_KEY_CODE_META_R];
+} else {
+[self toggleKey:Q_KEY_CODE_ALT_R];
+}
 }
 break;
 
@@ -861,14 +881,22 @@ - (bool) handleEventLocked:(NSEvent *)event
 case kVK_Command:
 if (isMouseGrabbed &&
 !!(modifiers & NSEventModifierFlagCommand)) {
-[self toggleKey:Q_KEY_CODE_META_L];
+if (swap_command_option_keys) {
+[self toggleKey:Q_KEY_CODE_ALT];
+} else {
+[self toggleKey:Q_KEY_CODE_META_L];
+}
 }
 break;
 
 case kVK_RightCommand:
 if (isMouseGrabbed &&
 !!(modifiers & NSEventModifierFlagCommand)) {
-[self toggleKey:Q_KEY_CODE_META_R];
+if (swap_command_option_keys) {
+[self toggleKey:Q_KEY_CODE_ALT_R];
+} else {
+[self toggleKey:Q_KEY_CODE_META_R];
+}
 }
 break;
 }
@@ -1188,6 +1216,7 @@ - (IBAction) do_about_menu_item: (id) sender;
 - (void)make_about_window;
 - (void)adjustSpeed:(id)sender;
 - (IBAction)doFullGrab:(id)sender;
+- (IBAction)doSwapCommandOptionKeys:(id)sender;
 @end
 
 @implementation QemuCocoaAppController
@@ -1669,6 +1698,22 @@ - (IBAction)doFullGrab:(id) sender
 }
 }
 
+// The action method to the "Options->Swap Command and Option Keys" menu item
+- (IBAction)doSwapCommandOptionKeys:(id)sender
+{
+// If the menu item is not checked
+if ([sender state] == NSControlStateValueOff) {
+swap_command_option_keys = true;
+[sender setState: NSControlStateValueOn];
+}
+
+// If the menu item is checked

Re: [PATCH for-6.2 07/43] target/ppc: Set fault address in ppc_cpu_do_unaligned_access

[Cédric Le Goater]

On 7/29/21 2:46 AM, Richard Henderson wrote:
> We ought to have been recording the virtual address for reporting
> to the guest trap handler.
> 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Richard Henderson 

Reviewed-by: Cédric Le Goater 


> ---
>  target/ppc/excp_helper.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index a79a0ed465..0b2c6de442 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr 
> vaddr,
>  CPUPPCState *env = cs->env_ptr;
>  uint32_t insn;
>  
> +env->spr[SPR_DAR] = vaddr;
> +
>  /* Restore state and reload the insn we executed, for filling in DSISR.  
> */
>  cpu_restore_state(cs, retaddr, true);
>  insn = cpu_ldl_code(env, env->nip);
> 

Re: need help with my config

[Cédric Le Goater]

Hello,

On 7/30/21 3:25 PM, Philippe Mathieu-Daudé wrote:
> Cc'ing qemu-ppc@
> 
> On 7/30/21 6:25 AM, Lindsay Ryan wrote:
>> Hi
>> I'm trying to emulate some physical IBM Power 9's that we have. There
>> seems to be plenty of examples of using x86_64 qemu, but slightly less
>> for Power. 

For baremetal emulation, please use the PowerNV machine. See this page : 

  https://qemu.readthedocs.io/en/latest/system/ppc/powernv.html

HW is not fully emulated but QEMU has enough support to start a multichip
system running any distro.  

>> Unless it's specifically for installing AIX

AIX only runs under the pseries machine (virtualized) and not on baremetal. 

>> Anyway, I'm trying to boot the VM as I guess a bare metal Power 9 box,
>> then install redhat from Iso on a disk and have it on the network.
>>
>> ./qemu-system-ppc64 -cpu POWER9 -smp cpus=4 -machine pseries -m 4096 -M
>> accel=tcg  -serial stdio -nodefaults -nographic -device
>> megasas,id=scsi0,bus=pci.0,addr=0x5 -drive
>> file=/home/hdisk1.qcow2,if=none,id=drive-scsi0-0-0-0,format=qcow2,cache=none
>> -device
>> scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2
>> -cdrom /mnt/images/rhel-8.4-ppc64le-boot.iso -monitor
>> telnet:0.0.0.0:3344,server,nowait -netdev
>> bridge,id=net0,helper=qemu-bridge-helper,br=bridge0,id=hostnet0

This is a pseries machine (virtualized) and not baremetal.

Which machine do you want to run ? pseries is the VM platform as run by KVM, 
It can run under TCG also. PowerNV is the baremetal platform on which KVM 
runs using the OPAL firmware. QEMU only has a PowerNV emulator, so TCG. 

These are two very different PPC machines.

>> So the megasas gets detected as a raid controller. Yay. 
>> But my qcow2 disk image doesn't seem to be plugged into it correctly as
>> it's not detected. 
>> It sees the cdrom image and I can boot from it.
>> The other thing I can't get working is the network card. 

because you don't have any :) Add a device and link it to the netdev.


That's how I run a TCG pseries POWER9 machine on my x86 laptop:

qemu-system-ppc64 -M 
pseries,cap-cfpc=workaround,cap-sbbc=workaround,cap-ibs=workaround,cap-ccf-assist=on,ic-mode=dual
 -m 4G -accel tcg,thread=multi -cpu POWER9 -smp 4,cores=4,maxcpus=8,threads=1 
-device virtio-net-pci,netdev=net0,mac=C0:FF:EE:00:00:02,bus=pci.0,addr=0x2 
-netdev tap,id=net0,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,vhost=on 
-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x1 -drive 
file=./ubuntu-ppc64le.qcow2,if=none,id=drive-scsi0-0-0-0,format=qcow2,cache=none
 -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1
 -device qemu-xhci,id=usb,bus=pci.0,addr=0x4 -nographic -nodefaults -serial 
mon:stdio

and a PowerNV POWER9 machine :

qemu-system-ppc64 -m 4G -machine powernv9 -smp 2 -accel tcg,thread=multi 
-kernel ./open-power/images/witherspoon-latest/zImage.epapr -initrd 
./open-power/images/witherspoon-latest/rootfs.cpio.xz -bios 
./open-power/images/witherspoon-latest/skiboot.lid -device 
pcie-pci-bridge,id=bridge1,bus=pcie.1,addr=0x0 -device 
ich9-ahci,id=sata0,bus=pcie.0,addr=0x0 -drive 
file=./ubuntu-ppc64le-powernv.qcow2,if=none,id=drive0,format=qcow2,cache=none 
-device ide-hd,bus=sata0.0,unit=0,drive=drive0,id=ide,bootindex=1 -device 
e1000e,netdev=net0,mac=C0:FF:EE:00:01:03,bus=bridge1,addr=0x3 -netdev 
bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 -device 
nec-usb-xhci,bus=bridge1,addr=0x2 -device usb-storage,drive=usbkey -drive 
file=./usb.img,if=none,id=usbkey,format=raw,cache=none -serial mon:stdio 
-nographic

You will need firmware images for the latter.


Cheers,
C.

Re: Problem with trace of x86 binary on x86_64 system with PANDA

[Peter Maydell]

On Fri, 30 Jul 2021 at 17:19, Nikita Gnilozub-Volobuev
 wrote:
> I'm a student and I'm writing my term paper. Part of it is the record
> the trace of x86 ELF on x86_64 Linux. For record I use PANDA.

Hi; PANDA is based on a fork of an older version of QEMU, with
considerable alterations. You are probably best off asking your
question on a PANDA specific mailing list or forum.

thanks
-- PMM

Problem with trace of x86 binary on x86_64 system with PANDA

[Nikita Gnilozub-Volobuev]

Hello there.

I'm a student and I'm writing my term paper. Part of it is the record 
the trace of x86 ELF on x86_64 Linux. For record I use PANDA. And I 
stacked with very strange problem: there is no system calls in my trace.
When I see this I was very surprised and make simple grabber of 
translation blocks. As I know I must see in that sysenter (0x0F 0x34) 
and int 0x80 (0xCD 0x80). But nothing of them was in my blocks. Maybe 
trouble is in TCG? How I can test this and proxing system calls through 
PANDA for analyze it?
For example what I mean: I have very stupid binary, it's just write 
"Hello world" to file. But PANDA can't catch any calls. Even write 
although it is clear that this call must be there.


--
With best wishes, Gnilozub-Volobuev N.I.

Re: "make check-acceptance" takes way too long

[Peter Maydell]

On Fri, 30 Jul 2021 at 16:12, Peter Maydell  wrote:
>
> "make check-acceptance" takes way way too long. I just did a run
> on an arm-and-aarch64-targets-only debug build and it took over
> half an hour, and this despite it skipping or cancelling 26 out
> of 58 tests!
>
> I think that ~10 minutes runtime is reasonable. 30 is not;
> ideally no individual test would take more than a minute or so.

Side note, can check-acceptance run multiple tests in parallel?
Running 3 or 4 at once would also improve the runtime...

-- PMM

Re: "make check-acceptance" takes way too long

[Philippe Mathieu-Daudé]

On 7/30/21 5:12 PM, Peter Maydell wrote:
> "make check-acceptance" takes way way too long. I just did a run
> on an arm-and-aarch64-targets-only debug build and it took over
> half an hour, and this despite it skipping or cancelling 26 out
> of 58 tests!
> 
> I think that ~10 minutes runtime is reasonable. 30 is not;
> ideally no individual test would take more than a minute or so.
> 
> Output saying where the time went. The first two tests take
> more than 10 minutes *each*. I think a good start would be to find
> a way of testing what they're testing that is less heavyweight.

IIRC the KVM forum BoF, we suggested a test shouldn't take more than
60sec. But then it was borderline for some tests so we talked about
allowing 90-120sec, and more should be discussed and documented.

However it was never documented / enforced.

This seems to match my memory:

$ git grep 'timeout =' tests/acceptance/
tests/acceptance/avocado_qemu/__init__.py:440:timeout = 900
tests/acceptance/boot_linux_console.py:99:timeout = 90
tests/acceptance/boot_xen.py:26:timeout = 90
tests/acceptance/linux_initrd.py:27:timeout = 300
tests/acceptance/linux_ssh_mips_malta.py:26:timeout = 150 # Not for
'configure --enable-debug --enable-debug-tcg'
tests/acceptance/machine_arm_canona1100.py:18:timeout = 90
tests/acceptance/machine_arm_integratorcp.py:34:timeout = 90
tests/acceptance/machine_arm_n8x0.py:20:timeout = 90
tests/acceptance/machine_avr6.py:25:timeout = 5
tests/acceptance/machine_m68k_nextcube.py:30:timeout = 15
tests/acceptance/machine_microblaze.py:14:timeout = 90
tests/acceptance/machine_mips_fuloong2e.py:18:timeout = 60
tests/acceptance/machine_mips_loongson3v.py:18:timeout = 60
tests/acceptance/machine_mips_malta.py:38:timeout = 30
tests/acceptance/machine_ppc.py:14:timeout = 90
tests/acceptance/machine_rx_gdbsim.py:22:timeout = 30
tests/acceptance/machine_s390_ccw_virtio.py:24:timeout = 120
tests/acceptance/machine_sparc64_sun4u.py:20:timeout = 90
tests/acceptance/machine_sparc_leon3.py:15:timeout = 60
tests/acceptance/migration.py:27:timeout = 10
tests/acceptance/ppc_prep_40p.py:18:timeout = 60
tests/acceptance/replay_kernel.py:34:timeout = 120
tests/acceptance/replay_kernel.py:357:timeout = 180
tests/acceptance/reverse_debugging.py:33:timeout = 10
tests/acceptance/tcg_plugins.py:24:timeout = 120

> 
>  (01/58) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv2:
> PASS (629.74 s)
>  (02/58) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv3:
> PASS (628.75 s)
>  (03/58) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm:
> CANCEL: kvm accelerator does not seem to be available (1.18 s)

We could restrict these to one of the projects runners (x86 probably)
with something like:

  @skipUnless(os.getenv('X86_64_RUNNER_AVAILABLE'), '...')

>  (15/58) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi:
> PASS (4.86 s)
>  (16/58) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd:
> PASS (39.85 s)
>  (17/58) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd:
> PASS (53.57 s)
>  (18/58) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08:
> SKIP: storage limited
>  (19/58) 
> tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
> SKIP: storage limited

I've been thinking about restricting them to my sdmmc-tree, but if
I don't send pull-req I won't test or catch other introducing
regressions. They respect the 60sec limit.

We could restrict some jobs to maintainers fork namespace, track
mainstream master branch and either run the pipelines when /master
is updated or regularly
(https://docs.gitlab.com/ee/ci/pipelines/schedules.html)
but them if the maintainer becomes busy / idle / inactive we
similarly won't catch regressions in mainstream.

Anyway Daniel already studied the problem and send a RFC but we
ignored it:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg761087.html

Maybe worth continuing the discussion there?

Re: [PATCH] gitlab-ci.d/buildtest: Mark the aarch64 and ppc64-s390x CFI jobs as manual

[Daniele Buono]

I agree, making these manual tasks until we find a fix for this is the 
only solution I can think of too.


Daniele

On 7/28/2021 3:51 AM, Thomas Huth wrote:

These two jobs are currently failing very often - the linker seems to
get killed due to out-of-memory problems. Since apparently nobody has
currently an idea how to fix that nicely, let's mark the jobs as manual
for the time being until someone comes up with a proper fix.

Signed-off-by: Thomas Huth 
---
  .gitlab-ci.d/buildtest.yml | 12 
  1 file changed, 12 insertions(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 63f1903f07..3537c6f1a1 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -416,6 +416,12 @@ build-cfi-aarch64:
  expire_in: 2 days
  paths:
- build
+  rules:
+# FIXME: This job is often failing, likely due to out-of-memory problems in
+# the constraint containers of the shared runners. Thus this is marked as
+# manual until the situation has been solved.
+- when: manual
+  allow_failure: true

  check-cfi-aarch64:
extends: .native_test_job_template
@@ -452,6 +458,12 @@ build-cfi-ppc64-s390x:
  expire_in: 2 days
  paths:
- build
+  rules:
+# FIXME: This job is often failing, likely due to out-of-memory problems in
+# the constraint containers of the shared runners. Thus this is marked as
+# manual until the situation has been solved.
+- when: manual
+  allow_failure: true

  check-cfi-ppc64-s390x:
extends: .native_test_job_template

[PATCH 2/2] target/arm: Implement M-profile trapping on division by zero

[Peter Maydell]

Unlike A-profile, for M-profile the UDIV and SDIV insns can be
configured to raise an exception on division by zero, using the CCR
DIV_0_TRP bit.

Implement support for setting this bit by making the helper functions
raise the appropriate exception.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h   |  1 +
 target/arm/helper.h|  4 ++--
 target/arm/helper.c| 19 +--
 target/arm/m_helper.c  |  4 
 target/arm/translate.c |  4 ++--
 5 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9f0a5f84d50..5cf8996ae3c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -54,6 +54,7 @@
 #define EXCP_LAZYFP 20   /* v7M fault during lazy FP stacking */
 #define EXCP_LSERR  21   /* v8M LSERR SecureFault */
 #define EXCP_UNALIGNED  22   /* v7M UNALIGNED UsageFault */
+#define EXCP_DIVBYZERO  23   /* v7M DIVBYZERO UsageFault */
 /* NB: add new EXCP_ defines to the array in arm_log_exception() too */
 
 #define ARMV7M_EXCP_RESET   1
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 248569b0cd8..aee8f0019b4 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -6,8 +6,8 @@ DEF_HELPER_3(add_saturate, i32, env, i32, i32)
 DEF_HELPER_3(sub_saturate, i32, env, i32, i32)
 DEF_HELPER_3(add_usaturate, i32, env, i32, i32)
 DEF_HELPER_3(sub_usaturate, i32, env, i32, i32)
-DEF_HELPER_FLAGS_2(sdiv, TCG_CALL_NO_RWG_SE, s32, s32, s32)
-DEF_HELPER_FLAGS_2(udiv, TCG_CALL_NO_RWG_SE, i32, i32, i32)
+DEF_HELPER_FLAGS_3(sdiv, TCG_CALL_NO_RWG, s32, env, s32, s32)
+DEF_HELPER_FLAGS_3(udiv, TCG_CALL_NO_RWG, i32, env, i32, i32)
 DEF_HELPER_FLAGS_1(rbit, TCG_CALL_NO_RWG_SE, i32, i32)
 
 #define PAS_OP(pfx)  \
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 8e9c2a2cf8c..56c520cf8e9 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9345,6 +9345,18 @@ uint32_t HELPER(sxtb16)(uint32_t x)
 return res;
 }
 
+static void handle_possible_div0_trap(CPUARMState *env, uintptr_t ra)
+{
+/*
+ * Take a division-by-zero exception if necessary; otherwise return
+ * to get the usual non-trapping division behaviour (result of 0)
+ */
+if (arm_feature(env, ARM_FEATURE_M)
+&& (env->v7m.ccr[env->v7m.secure] & R_V7M_CCR_DIV_0_TRP_MASK)) {
+raise_exception_ra(env, EXCP_DIVBYZERO, 0, 1, ra);
+}
+}
+
 uint32_t HELPER(uxtb16)(uint32_t x)
 {
 uint32_t res;
@@ -9353,9 +9365,10 @@ uint32_t HELPER(uxtb16)(uint32_t x)
 return res;
 }
 
-int32_t HELPER(sdiv)(int32_t num, int32_t den)
+int32_t HELPER(sdiv)(CPUARMState *env, int32_t num, int32_t den)
 {
 if (den == 0) {
+handle_possible_div0_trap(env, GETPC());
 return 0;
 }
 if (num == INT_MIN && den == -1) {
@@ -9364,9 +9377,10 @@ int32_t HELPER(sdiv)(int32_t num, int32_t den)
 return num / den;
 }
 
-uint32_t HELPER(udiv)(uint32_t num, uint32_t den)
+uint32_t HELPER(udiv)(CPUARMState *env, uint32_t num, uint32_t den)
 {
 if (den == 0) {
+handle_possible_div0_trap(env, GETPC());
 return 0;
 }
 return num / den;
@@ -9567,6 +9581,7 @@ void arm_log_exception(int idx)
 [EXCP_LAZYFP] = "v7M exception during lazy FP stacking",
 [EXCP_LSERR] = "v8M LSERR UsageFault",
 [EXCP_UNALIGNED] = "v7M UNALIGNED UsageFault",
+[EXCP_DIVBYZERO] = "v7M DIVBYZERO UsageFault",
 };
 
 if (idx >= 0 && idx < ARRAY_SIZE(excnames)) {
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 20761c94877..47903b3dc35 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -2252,6 +2252,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure);
 env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_UNALIGNED_MASK;
 break;
+case EXCP_DIVBYZERO:
+armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure);
+env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_DIVBYZERO_MASK;
+break;
 case EXCP_SWI:
 /* The PC already points to the next instruction.  */
 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC, env->v7m.secure);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 80c282669f0..28eabeb2323 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -7992,9 +7992,9 @@ static bool op_div(DisasContext *s, arg_rrr *a, bool u)
 t1 = load_reg(s, a->rn);
 t2 = load_reg(s, a->rm);
 if (u) {
-gen_helper_udiv(t1, t1, t2);
+gen_helper_udiv(t1, cpu_env, t1, t2);
 } else {
-gen_helper_sdiv(t1, t1, t2);
+gen_helper_sdiv(t1, cpu_env, t1, t2);
 }
 tcg_temp_free_i32(t2);
 store_reg(s, a->rd, t1);
-- 
2.20.1

[PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers

[Peter Maydell]

We're about to make a code change to the sdiv and udiv helper
functions, so first fix their indentation and coding style.

Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 155d8bf2399..8e9c2a2cf8c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9355,17 +9355,20 @@ uint32_t HELPER(uxtb16)(uint32_t x)
 
 int32_t HELPER(sdiv)(int32_t num, int32_t den)
 {
-if (den == 0)
-  return 0;
-if (num == INT_MIN && den == -1)
-  return INT_MIN;
+if (den == 0) {
+return 0;
+}
+if (num == INT_MIN && den == -1) {
+return INT_MIN;
+}
 return num / den;
 }
 
 uint32_t HELPER(udiv)(uint32_t num, uint32_t den)
 {
-if (den == 0)
-  return 0;
+if (den == 0) {
+return 0;
+}
 return num / den;
 }
 
-- 
2.20.1

[PATCH 0/2] arm: Implement M-profile trapping on division by zero

[Peter Maydell]

Unlike A-profile, for M-profile the UDIV and SDIV insns can be
configured to raise an exception on division by zero, using the CCR
DIV_0_TRP bit.  This patchset implements that missing functionality
by having the udiv and sdiv helpers raise an exception if needed.

Some questions:

Is it worth allowing A-profile to retain the mildly better codegen it
gets from not having to pass in 'env' and marking the helper as
no-side-effects (ie having M-specific udiv/sdiv helpers) ?

Is it worth inlining either udiv or sdiv for the A-profile case?
udiv can be done with movcond/movcond/divu, something like:

/* t1 = (t2 == 0) ? 0 : t1;t2 = (t2 == 0) ? 1 : t2 */
tcg_gen_movcond_i32(TCG_COND_EQ, t1, t2, tcg_constant_i32(0),
tcg_constant_i32(0), t1);
tcg_gen_movcond_i32(TCG_COND_EQ, t2, t2, tcg_constant_i32(0),
tcg_constant_i32(1), t2);
/* Either t1 / t2; or 0 / 1 to give 0 for division-by-zero */
tcg_gen_divu_i32(t1, t1, t2);

sdiv is more painful because it needs to check for both x/0 and
INTMIN/-1 cases.  Some other targets choose to generate inline TCG
ops for it, though.

Side note, I don't understand the x86-64 codegen for the above
sketch of an inline udiv. When I try it the TCG ops are

  mov_i32 tmp3,r2
  mov_i32 tmp6,r3
  movcond_i32 tmp3,tmp6,$0x0,$0x0,tmp3,eq
  movcond_i32 tmp6,tmp6,$0x0,$0x1,tmp6,eq
  mov_i32 tmp7,$0x0
  divu2_i32 tmp3,tmp7,tmp3,tmp7,tmp6
  mov_i32 r3,tmp3

but the x86 code is
0x7f5f1807dc0c:  45 33 f6 xorl %r14d, %r14d
0x7f5f1807dc0f:  45 85 ed testl%r13d, %r13d
0x7f5f1807dc12:  45 0f 44 e6  cmovel   %r14d, %r12d
0x7f5f1807dc16:  41 bf 01 00 00 00movl $1, %r15d
0x7f5f1807dc1c:  45 3b ee cmpl %r14d, %r13d
0x7f5f1807dc1f:  45 0f 44 ef  cmovel   %r15d, %r13d
0x7f5f1807dc23:  41 8b c4 movl %r12d, %eax
0x7f5f1807dc26:  41 8b d6 movl %r14d, %edx
0x7f5f1807dc29:  41 f7 f5 divl %r13d

where the comparison for the first cmovel is 'testl %r13d, %r13d",
but the second comparison is 'cmpl %r14d, %r13d'.  That's the same
effect (given r14 is 0) but I don't understand why the backend has
chosen to generate different code for the two cases.  (Ideally of
course it would notice that it already had generated the condition
check and not repeat it.)

thanks
-- PMM

Peter Maydell (2):
  target/arm: Re-indent sdiv and udiv helpers
  target/arm: Implement M-profile trapping on division by zero

 target/arm/cpu.h   |  1 +
 target/arm/helper.h|  4 ++--
 target/arm/helper.c| 34 ++
 target/arm/m_helper.c  |  4 
 target/arm/translate.c |  4 ++--
 5 files changed, 35 insertions(+), 12 deletions(-)

-- 
2.20.1

Re: [PATCH v2 4/6] util/oslib-posix: Avoid creating a single thread with MADV_POPULATE_WRITE

[David Hildenbrand]

On 27.07.21 21:04, Dr. David Alan Gilbert wrote:

* David Hildenbrand (da...@redhat.com) wrote:

Let's simplify the case when we only want a single thread and don't have
to mess with signal handlers.

Signed-off-by: David Hildenbrand 
---
  util/oslib-posix.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index a1d309d495..1483e985c6 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -568,6 +568,14 @@ static bool touch_all_pages(char *area, size_t hpagesize, 
size_t numpages,
  }
  
  if (use_madv_populate_write) {

+/* Avoid creating a single thread for MADV_POPULATE_WRITE */
+if (context.num_threads == 1) {
+if (qemu_madvise(area, hpagesize * numpages,
+ QEMU_MADV_POPULATE_WRITE)) {


Do you never have to fall back if this particular memory region is the
one that can't do madv?


We sense upfront, when detecting use_madv_populate_write, whether it's 
supported on this very memory type. So, no need to fallback here.




Dave


--
Thanks,

David / dhildenb

"make check-acceptance" takes way too long

[Peter Maydell]

"make check-acceptance" takes way way too long. I just did a run
on an arm-and-aarch64-targets-only debug build and it took over
half an hour, and this despite it skipping or cancelling 26 out
of 58 tests!

I think that ~10 minutes runtime is reasonable. 30 is not;
ideally no individual test would take more than a minute or so.

Output saying where the time went. The first two tests take
more than 10 minutes *each*. I think a good start would be to find
a way of testing what they're testing that is less heavyweight.

 (01/58) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv2:
PASS (629.74 s)
 (02/58) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv3:
PASS (628.75 s)
 (03/58) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm:
CANCEL: kvm accelerator does not seem to be available (1.18 s)
 (04/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_aarch64_virt:
PASS (3.53 s)
 (05/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_aarch64_xlnx_versal_virt:
PASS (41.13 s)
 (06/58) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_virt:
PASS (5.22 s)
 (07/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2:
PASS (18.88 s)
 (08/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_raspi2_uart0:
PASS (11.30 s)
 (09/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_raspi2_initrd:
PASS (22.66 s)
 (10/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_exynos4210_initrd:
PASS (31.89 s)
 (11/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_initrd:
PASS (27.86 s)
 (12/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_sata:
PASS (27.19 s)
 (13/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_quanta_gsj:
SKIP: Test might timeout
 (14/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_quanta_gsj_initrd:
PASS (22.53 s)
 (15/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi:
PASS (4.86 s)
 (16/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_initrd:
PASS (39.85 s)
 (17/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd:
PASS (53.57 s)
 (18/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08:
SKIP: storage limited
 (19/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
SKIP: storage limited
 (20/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_aarch64_raspi3_atf:
PASS (1.50 s)
 (21/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_vexpressa9:
PASS (10.74 s)
 (22/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_ast2400_palmetto_openbmc_v2_9_0:
PASS (39.43 s)
 (23/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_ast2500_romulus_openbmc_v2_9_0:
PASS (54.01 s)
 (24/58) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_ast2600_debian:
PASS (40.60 s)
 (25/58) tests/acceptance/boot_xen.py:BootXen.test_arm64_xen_411_and_dom0:
PASS (20.22 s)
 (26/58) tests/acceptance/boot_xen.py:BootXen.test_arm64_xen_414_and_dom0:
PASS (17.37 s)
 (27/58) tests/acceptance/boot_xen.py:BootXen.test_arm64_xen_415_and_dom0:
PASS (23.82 s)
 (28/58) tests/acceptance/empty_cpu_model.py:EmptyCPUModel.test:
CANCEL: No QEMU binary defined or found in the build tree (0.00 s)
 (29/58) tests/acceptance/info_usernet.py:InfoUsernet.test_hostfwd:
CANCEL: No QEMU binary defined or found in the build tree (0.00 s)
 (30/58) 
tests/acceptance/machine_arm_canona1100.py:CanonA1100Machine.test_arm_canona1100:
PASS (0.20 s)
 (31/58) 
tests/acceptance/machine_arm_integratorcp.py:IntegratorMachine.test_integratorcp_console:
SKIP: untrusted code
 (32/58) 
tests/acceptance/machine_arm_integratorcp.py:IntegratorMachine.test_framebuffer_tux_logo:
SKIP: Python NumPy not installed
 (33/58) tests/acceptance/machine_arm_n8x0.py:N8x0Machine.test_n800:
SKIP: untrusted code
 (34/58) tests/acceptance/machine_arm_n8x0.py:N8x0Machine.test_n810:
SKIP: untrusted code
 (35/58) 
tests/acceptance/migration.py:Migration.test_migration_with_tcp_localhost:
CANCEL: No QEMU binary defined or found in the build tree (0.00 s)
 (36/58) tests/acceptance/migration.py:Migration.test_migration_with_unix:
CANCEL: No QEMU binary defined or found in the build tree (0.00 s)
 (37/58) tests/acceptance/migration.py:Migration.test_migration_with_exec:
CANCEL: No QEMU binary defined or found in the build tree (0.00 s)
 (38/58) 
tests/acceptance/multiprocess.py:Multiprocess.test_multiprocess_aarch64:
CANCEL: kvm accelerator does not seem to be available (0.06 s)
 (39/58) tests/acceptance/replay_kernel.py:ReplayKernelNormal.test_aarch64_virt:
PASS (19.59 s)
 (40/58) tests/acceptance/replay_kernel.py:ReplayKernelNormal.test_arm_virt:
PASS (28.73 s)
 (41/58) 

Re: [PATCH RFC 0/3] mirror: rework soft-cancelling READY mirror

[Max Reitz]

On 29.07.21 18:29, Vladimir Sementsov-Ogievskiy wrote:

29.07.2021 16:47, Max Reitz wrote:

On 29.07.21 13:35, Vladimir Sementsov-Ogievskiy wrote:

29.07.2021 13:38, Max Reitz wrote:

On 29.07.21 12:02, Vladimir Sementsov-Ogievskiy wrote:

28.07.2021 10:00, Max Reitz wrote:

On 27.07.21 18:47, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

That's an alternative to (part of) Max's
"[PATCH for-6.1? v2 0/7] mirror: Handle errors after READY cancel"
and shows' my idea of handling soft-cancelling READY mirror case
directly in qmp_block_job_cancel. And cleanup all other job 
cancelling

functions.

That's untested draft, don't take it to heart :)


Well, I would have preferred it if you’d rebased this on top of 
that series, precisely because it’s an alternative to only part 
of it. And if it’s just an untested draft, that would have been 
even better, because it would’ve given a better idea on what the 
cleanup looks like.


There are also things like this series making cancel internally 
always a force-cancel, where I’m not sure whether we want that in 
the replication driver or not[1].  With my series, we add an 
explicit parameter, so we’re forced to think about it, and then 
in this series on top we can just drop the parameter for all 
force-cancel invocations again, and for all non-force-cancel 
invocations we would have to think a bit more.


I now don't sure that patch 5 of your series is correct (see my 
last answer to it), that's why I decided to not base on it.


Well, we can always take patch 5 from v1.  (Where I changed any 
job_is_cancelled() to job_cancel_requested() when it influenced the 
external interface.)


My series has the benefit of handling soft-mirror-cancel case the 
other way and handles mirror finalization in case of soft-cancel 
properly.




Specifically as for this series, I don’t like job_complete_ex() 
very much, I think the parameter should be part of job_complete() 
itself.


That was my idea. But job_complete is passed as function pointer, 
so changing its prototype would be more work.. But I think it's 
possible.


  If we think that’s too specific of a mirror parameter to 
include in normal job_complete(), well, then there shouldn’t be a 
job_complete_ex() either, and do_graph_change should be a 
property of the mirror job (perhaps as pivot_on_completion) 
that’s cleared by qmp_block_job_cancel() before invoking 
job_complete().


This way users will lose a way to make a decision during job 
running..


On the contrary, it would be a precursor to letting the user change 
this property explicitly with a new QMP command.


But probably they don't need actually. Moving the option to mirror 
job parameter seems a good option to me.




Max

[1] Although looking at it again now, it probably wants 
force-cancel.





What do you think of my idea to keep old bugs as is and just 
deprecate block-job-cancel and add a new interface for 
"no-graph-change mirror" case?


I don’t see a reason for that.  The fix isn’t that complicated.

Also, honestly, I don’t see a good reason for deprecating anything.



Current interface lead to mess in the code, that's bad. Cancellation 
mode that is actually a kind of completion (and having comments in 
many places about that) - that shows for me that interface is not 
good.. It's a question of terminology, what to call "cancel". Also, 
that's not the first time this question arise. Remember my recent 
cancel-in-flight-requests series, when I thought that "cancel is 
cancel" and didn't consider soft-cancel of mirror.. And reviewers 
didn't caught it. I don't think that interface is good, it will 
always confuse new developers and users. But that's just my opinion, 
I don't impose it )


If not deprecate, i.e. if we consider old interface to be good, than 
no reason for this my series and for introducing new interface :)


I’m not against a better interface, I’m against using this current 
bug as an excuse to improve the interface.  We’ve known we want to 
improve the interface for quite a long time now, we don’t need an 
excuse for that.


If we use this bug as an excuse, I’m afraid of becoming hung up on 
interface discussions instead of just getting the bug fixed. And we 
must get the bug fixed, it’s real, it’s kind of bad, and saying “it 
won’t appear with the new interface, let’s not worry about the old 
one” is not something I like.


OTOH, if we use this bug as an excuse, I’m also afraid of trying to 
rush the design instead of actually implementing the interface that 
we’ve always desired, i.e. where the user gets to choose the 
completion mode via yet-to-be-implemented some job property setter 
function.


As a final note (but this is precisely the interface discussion that 
I want to avoid for now), I said I don’t see a good reason for 
deprecating anything, because `job-cancel force=false` can just 
internally do `set-job-property .pivot_on_completion=false; 
job-complete`.  From an implementation perspective, that should be 
simple.


I 

Re: need help with my config

[Klaus Kiwi]

Just making sure Cedric is also seeing this...

 -Klaus

On Fri, Jul 30, 2021 at 10:27 AM Philippe Mathieu-Daudé 
wrote:

> Cc'ing qemu-ppc@
>
> On 7/30/21 6:25 AM, Lindsay Ryan wrote:
> > Hi
> > I'm trying to emulate some physical IBM Power 9's that we have. There
> > seems to be plenty of examples of using x86_64 qemu, but slightly less
> > for Power. Unless it's specifically for installing AIX
> > Anyway, I'm trying to boot the VM as I guess a bare metal Power 9 box,
> > then install redhat from Iso on a disk and have it on the network.
> >
> > ./qemu-system-ppc64 -cpu POWER9 -smp cpus=4 -machine pseries -m 4096 -M
> > accel=tcg  -serial stdio -nodefaults -nographic -device
> > megasas,id=scsi0,bus=pci.0,addr=0x5 -drive
> >
> file=/home/hdisk1.qcow2,if=none,id=drive-scsi0-0-0-0,format=qcow2,cache=none
> > -device
> >
> scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2
> > -cdrom /mnt/images/rhel-8.4-ppc64le-boot.iso -monitor
> > telnet:0.0.0.0:3344,server,nowait -netdev
> > bridge,id=net0,helper=qemu-bridge-helper,br=bridge0,id=hostnet0
> >
> > So the megasas gets detected as a raid controller. Yay.
> > But my qcow2 disk image doesn't seem to be plugged into it correctly as
> > it's not detected.
> > It sees the cdrom image and I can boot from it.
> > The other thing I can't get working is the network card.
> >
> > On this host system I have some other x86 kvm's running. So Ideally if I
> > could plug the nic on this vm into
> > 7: virbr0
> >
> > 6: bridge0:  mtu 1500 qdisc noqueue
> > state UP group default qlen 1000
> > link/ether 00:25:b5:04:2a:1e brd ff:ff:ff:ff:ff:ff
> > inet 10.126.24.82/24 brd 10.126.24.255 scope global noprefixroute
> > bridge0
> >valid_lft forever preferred_lft forever
> > inet6 fe80::76a8:89ec:fc62:9c94/64 scope link noprefixroute
> >valid_lft forever preferred_lft forever
> > 7: virbr0:  mtu 1500 qdisc noqueue
> > state UP group default qlen 1000
> > link/ether 52:54:00:51:db:be brd ff:ff:ff:ff:ff:ff
> > inet 192.168.122.1/24 brd 192.168.122.255 scope global virbr0
> >valid_lft forever preferred_lft forever
> >
> > If I can't do that, then I really only need the ppc64 guest to have
> > access out to the internet and I can nat anything incoming
> >
> > Any help, particularly about how to plug virtual disk drives into
> > virtual disk controllers would be helpful
> > regards
> >
> >
> > Ryan Lindsay BEng, MSc.
> > Linux Storage Administrator
> > Research Computing Facility
> >
> >
> > *Disclaimer: *This email (including any attachments or links) may
> > contain confidential and/or legally privileged information and is
> > intended only to be read or used by the addressee. If you are not the
> > intended addressee, any use, distribution, disclosure or copying of this
> > email is strictly prohibited. Confidentiality and legal privilege
> > attached to this email (including any attachments) are not waived or
> > lost by reason of its mistaken delivery to you. If you have received
> > this email in error, please delete it and notify us immediately by
> > telephone or email. Peter MacCallum Cancer Centre provides no guarantee
> > that this transmission is free of virus or that it has not been
> > intercepted or altered and will not be liable for any delay in its
> receipt.
> >
>
>
>

-- 
Klaus Heinrich Kiwi 
Manager, Software Engineering - Red Hat Virtualization

[PATCH v3 06/10] virtiofsd: Let lo_inode_open() return a TempFd

[Max Reitz]

Strictly speaking, this is not necessary, because lo_inode_open() will
always return a new FD owned by the caller, so TempFd.owned will always
be true.

However, auto-cleanup is nice, and in some cases this plays nicely with
an lo_inode_fd() call in another conditional branch (see lo_setattr()).

Signed-off-by: Max Reitz 
---
 tools/virtiofsd/passthrough_ll.c | 138 +--
 1 file changed, 59 insertions(+), 79 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 9e1bc37af8..292b7f7e27 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -291,10 +291,8 @@ static void temp_fd_clear(TempFd *temp_fd)
 /**
  * Return an owned fd from *temp_fd that will not be closed when
  * *temp_fd goes out of scope.
- *
- * (TODO: Remove __attribute__ once this is used.)
  */
-static __attribute__((unused)) int temp_fd_steal(TempFd *temp_fd)
+static int temp_fd_steal(TempFd *temp_fd)
 {
 if (temp_fd->owned) {
 temp_fd->owned = false;
@@ -673,9 +671,12 @@ static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd 
*tfd)
  * when a malicious client opens special files such as block device nodes.
  * Symlink inodes are also rejected since symlinks must already have been
  * traversed on the client side.
+ *
+ * The fd is returned in tfd->fd.  The return value is 0 on success and -errno
+ * otherwise.
  */
-static int lo_inode_open(struct lo_data *lo, struct lo_inode *inode,
- int open_flags)
+static int lo_inode_open(const struct lo_data *lo, const struct lo_inode 
*inode,
+ int open_flags, TempFd *tfd)
 {
 g_autofree char *fd_str = g_strdup_printf("%d", inode->fd);
 int fd;
@@ -694,7 +695,13 @@ static int lo_inode_open(struct lo_data *lo, struct 
lo_inode *inode,
 if (fd < 0) {
 return -errno;
 }
-return fd;
+
+*tfd = (TempFd) {
+.fd = fd,
+.owned = true,
+};
+
+return 0;
 }
 
 static void lo_init(void *userdata, struct fuse_conn_info *conn)
@@ -852,7 +859,12 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 return;
 }
 
-res = lo_inode_fd(inode, _fd);
+if (!fi && (valid & FUSE_SET_ATTR_SIZE)) {
+/* We need an O_RDWR FD for ftruncate() */
+res = lo_inode_open(lo, inode, O_RDWR, _fd);
+} else {
+res = lo_inode_fd(inode, _fd);
+}
 if (res < 0) {
 saverr = -res;
 goto out_err;
@@ -900,18 +912,11 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 if (fi) {
 truncfd = fd;
 } else {
-truncfd = lo_inode_open(lo, inode, O_RDWR);
-if (truncfd < 0) {
-saverr = -truncfd;
-goto out_err;
-}
+truncfd = inode_fd.fd;
 }
 
 saverr = drop_security_capability(lo, truncfd);
 if (saverr) {
-if (!fi) {
-close(truncfd);
-}
 goto out_err;
 }
 
@@ -919,9 +924,6 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 res = drop_effective_cap("FSETID", _fsetid_dropped);
 if (res != 0) {
 saverr = res;
-if (!fi) {
-close(truncfd);
-}
 goto out_err;
 }
 }
@@ -934,9 +936,6 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 fuse_log(FUSE_LOG_ERR, "Failed to gain CAP_FSETID\n");
 }
 }
-if (!fi) {
-close(truncfd);
-}
 if (res == -1) {
 goto out_err;
 }
@@ -1822,11 +1821,12 @@ static struct lo_dirp *lo_dirp(fuse_req_t req, struct 
fuse_file_info *fi)
 static void lo_opendir(fuse_req_t req, fuse_ino_t ino,
struct fuse_file_info *fi)
 {
+g_auto(TempFd) inode_fd = TEMP_FD_INIT;
 int error = ENOMEM;
 struct lo_data *lo = lo_data(req);
 struct lo_inode *inode;
 struct lo_dirp *d = NULL;
-int fd;
+int res;
 ssize_t fh;
 
 inode = lo_inode(req, ino);
@@ -1840,13 +1840,13 @@ static void lo_opendir(fuse_req_t req, fuse_ino_t ino,
 goto out_err;
 }
 
-fd = lo_inode_open(lo, inode, O_RDONLY);
-if (fd < 0) {
-error = -fd;
+res = lo_inode_open(lo, inode, O_RDONLY, _fd);
+if (res < 0) {
+error = -res;
 goto out_err;
 }
 
-d->dp = fdopendir(fd);
+d->dp = fdopendir(temp_fd_steal(_fd));
 if (d->dp == NULL) {
 goto out_errno;
 }
@@ -1876,8 +1876,6 @@ out_err:
 if (d) {
 if (d->dp) {
 closedir(d->dp);
-} else if (fd != -1) {
-close(fd);
 }
 free(d);
 }
@@ -2077,6 +2075,7 @@ static void update_open_flags(int writeback, int 
allow_direct_io,
 static int lo_do_open(struct lo_data *lo, 

[PATCH v3 10/10] virtiofsd: Add lazy lo_do_find()

[Max Reitz]

lo_find() right now takes two lookup keys for two maps, namely the file
handle for inodes_by_handle and the statx information for inodes_by_ids.
However, we only need the statx information if looking up the inode by
the file handle failed.

There are two callers of lo_find(): The first one, lo_do_lookup(), has
both keys anyway, so passing them does not incur any additional cost.
The second one, lookup_name(), though, needs to explicitly invoke
name_to_handle_at() (through get_file_handle()) and statx() (through
do_statx()).  We need to try to get a file handle as the primary key, so
we cannot get rid of get_file_handle(), but we only need the statx
information if looking up an inode by handle failed; so we can defer
that until the lookup has indeed failed.

To this end, replace lo_find()'s st/mnt_id parameters by a get_ids()
closure that is invoked to fill the lo_key struct if necessary.

Also, lo_find() is renamed to lo_do_find(), so we can add a new
lo_find() wrapper whose closure just initializes the lo_key from the
st/mnt_id parameters, just like the old lo_find() did.

lookup_name() directly calls lo_do_find() now and passes its own
closure, which performs the do_statx() call.

Signed-off-by: Max Reitz 
---
 tools/virtiofsd/passthrough_ll.c | 93 ++--
 1 file changed, 76 insertions(+), 17 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index ac95961d12..41e9f53878 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -1200,22 +1200,23 @@ out_err:
 fuse_reply_err(req, saverr);
 }
 
-static struct lo_inode *lo_find(struct lo_data *lo,
-const struct lo_fhandle *fhandle,
-struct stat *st, uint64_t mnt_id)
+/*
+ * get_ids() will be called to get the key for lo->inodes_by_ids if
+ * the lookup by file handle has failed.
+ */
+static struct lo_inode *lo_do_find(struct lo_data *lo,
+const struct lo_fhandle *fhandle,
+int (*get_ids)(struct lo_key *, const void *),
+const void *get_ids_opaque)
 {
 struct lo_inode *p = NULL;
-struct lo_key ids_key = {
-.ino = st->st_ino,
-.dev = st->st_dev,
-.mnt_id = mnt_id,
-};
+struct lo_key ids_key;
 
 pthread_mutex_lock(>mutex);
 if (fhandle) {
 p = g_hash_table_lookup(lo->inodes_by_handle, fhandle);
 }
-if (!p) {
+if (!p && get_ids(_key, get_ids_opaque) == 0) {
 p = g_hash_table_lookup(lo->inodes_by_ids, _key);
 /*
  * When we had to fall back to looking up an inode by its
@@ -1244,6 +1245,36 @@ static struct lo_inode *lo_find(struct lo_data *lo,
 return p;
 }
 
+struct lo_find_get_ids_key_opaque {
+const struct stat *st;
+uint64_t mnt_id;
+};
+
+static int lo_find_get_ids_key(struct lo_key *ids_key, const void *opaque)
+{
+const struct lo_find_get_ids_key_opaque *stat_info = opaque;
+
+*ids_key = (struct lo_key){
+.ino = stat_info->st->st_ino,
+.dev = stat_info->st->st_dev,
+.mnt_id = stat_info->mnt_id,
+};
+
+return 0;
+}
+
+static struct lo_inode *lo_find(struct lo_data *lo,
+const struct lo_fhandle *fhandle,
+struct stat *st, uint64_t mnt_id)
+{
+const struct lo_find_get_ids_key_opaque stat_info = {
+.st = st,
+.mnt_id = mnt_id,
+};
+
+return lo_do_find(lo, fhandle, lo_find_get_ids_key, _info);
+}
+
 /* value_destroy_func for posix_locks GHashTable */
 static void posix_locks_value_destroy(gpointer data)
 {
@@ -1769,14 +1800,41 @@ out_err:
 fuse_reply_err(req, saverr);
 }
 
+struct lookup_name_get_ids_key_opaque {
+struct lo_data *lo;
+int parent_fd;
+const char *name;
+};
+
+static int lookup_name_get_ids_key(struct lo_key *ids_key, const void *opaque)
+{
+const struct lookup_name_get_ids_key_opaque *stat_params = opaque;
+uint64_t mnt_id;
+struct stat attr;
+int res;
+
+res = do_statx(stat_params->lo, stat_params->parent_fd, stat_params->name,
+   , AT_SYMLINK_NOFOLLOW, _id);
+if (res < 0) {
+return -errno;
+}
+
+*ids_key = (struct lo_key){
+.ino = attr.st_ino,
+.dev = attr.st_dev,
+.mnt_id = mnt_id,
+};
+
+return 0;
+}
+
 /* Increments nlookup and caller must release refcount using lo_inode_put() */
 static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent,
 const char *name)
 {
 g_auto(TempFd) dir_fd = TEMP_FD_INIT;
 int res;
-uint64_t mnt_id;
-struct stat attr;
+struct lookup_name_get_ids_key_opaque stat_params;
 struct lo_fhandle *fh;
 struct lo_data *lo = lo_data(req);
 struct lo_inode *dir = lo_inode(req, parent);
@@ -1794,12 +1852,13 @@ static struct lo_inode *lookup_name(fuse_req_t req, 
fuse_ino_t parent,
 fh = get_file_handle(lo, dir_fd.fd, name);
 /* 

[PATCH v3 09/10] virtiofsd: Optionally fill lo_inode.fhandle

[Max Reitz]

When the inode_file_handles option is set, try to generate a file handle
for new inodes instead of opening an O_PATH FD.

Being able to open these again will require CAP_DAC_READ_SEARCH, so the
description text tells the user they will also need to specify
-o modcaps=+dac_read_search.

Generating a file handle returns the mount ID it is valid for.  Opening
it will require an FD instead.  We have mount_fds to map an ID to an FD.
get_file_handle() fills the hash map by opening the file we have
generated a handle for.  To verify that the resulting FD indeed
represents the handle's mount ID, we use statx().  Therefore, using file
handles requires statx() support.

Signed-off-by: Max Reitz 
---
 tools/virtiofsd/helper.c  |   3 +
 tools/virtiofsd/passthrough_ll.c  | 194 --
 tools/virtiofsd/passthrough_seccomp.c |   1 +
 3 files changed, 190 insertions(+), 8 deletions(-)

diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index a8295d975a..aa63a21d43 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -187,6 +187,9 @@ void fuse_cmdline_help(void)
"   default: no_allow_direct_io\n"
"-o announce_submounts  Announce sub-mount points to the 
guest\n"
"-o posix_acl/no_posix_acl  Enable/Disable posix_acl. (default: 
disabled)\n"
+   "-o inode_file_handles  Use file handles to reference 
inodes\n"
+   "   instead of O_PATH file 
descriptors\n"
+   "   (requires -o 
modcaps=+dac_read_search)\n"
);
 }
 
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index f9d8b2f134..ac95961d12 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -194,6 +194,7 @@ struct lo_data {
 /* If set, virtiofsd is responsible for setting umask during creation */
 bool change_umask;
 int user_posix_acl, posix_acl;
+int inode_file_handles;
 };
 
 /**
@@ -250,6 +251,10 @@ static const struct fuse_opt lo_opts[] = {
 { "no_killpriv_v2", offsetof(struct lo_data, user_killpriv_v2), 0 },
 { "posix_acl", offsetof(struct lo_data, user_posix_acl), 1 },
 { "no_posix_acl", offsetof(struct lo_data, user_posix_acl), 0 },
+{ "inode_file_handles", offsetof(struct lo_data, inode_file_handles), 1 },
+{ "no_inode_file_handles",
+  offsetof(struct lo_data, inode_file_handles),
+  0 },
 FUSE_OPT_END
 };
 static bool use_syslog = false;
@@ -321,6 +326,135 @@ static int temp_fd_steal(TempFd *temp_fd)
 }
 }
 
+/**
+ * Generate a file handle for the given dirfd/name combination.
+ *
+ * If mount_fds does not yet contain an entry for the handle's mount
+ * ID, (re)open dirfd/name in O_RDONLY mode and add it to mount_fds
+ * as the FD for that mount ID.  (That is the file that we have
+ * generated a handle for, so it should be representative for the
+ * mount ID.  However, to be sure (and to rule out races), we use
+ * statx() to verify that our assumption is correct.)
+ */
+static struct lo_fhandle *get_file_handle(struct lo_data *lo,
+  int dirfd, const char *name)
+{
+/* We need statx() to verify the mount ID */
+#if defined(CONFIG_STATX) && defined(STATX_MNT_ID)
+struct lo_fhandle *fh;
+int ret;
+
+if (!lo->use_statx || !lo->inode_file_handles) {
+return NULL;
+}
+
+fh = g_new0(struct lo_fhandle, 1);
+
+fh->handle.handle_bytes = sizeof(fh->padding) - sizeof(fh->handle);
+ret = name_to_handle_at(dirfd, name, >handle, >mount_id,
+AT_EMPTY_PATH);
+if (ret < 0) {
+goto fail;
+}
+
+if (pthread_rwlock_rdlock(_fds_lock)) {
+goto fail;
+}
+if (!g_hash_table_contains(mount_fds, GINT_TO_POINTER(fh->mount_id))) {
+g_auto(TempFd) path_fd = TEMP_FD_INIT;
+struct statx stx;
+char procname[64];
+int fd;
+
+pthread_rwlock_unlock(_fds_lock);
+
+/*
+ * Before opening an O_RDONLY fd, check whether dirfd/name is a regular
+ * file or directory, because we must not open anything else with
+ * anything but O_PATH.
+ * (And we use that occasion to verify that the file has the mount ID 
we
+ * need.)
+ */
+if (name[0]) {
+path_fd.fd = openat(dirfd, name, O_PATH);
+if (path_fd.fd < 0) {
+goto fail;
+}
+path_fd.owned = true;
+} else {
+path_fd.fd = dirfd;
+path_fd.owned = false;
+}
+
+ret = statx(path_fd.fd, "", AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW,
+STATX_TYPE | STATX_MNT_ID, );
+if (ret < 0) {
+if (errno == ENOSYS) {
+lo->use_statx = false;
+fuse_log(FUSE_LOG_WARNING,
+ "statx() does 

[PATCH v3 02/10] virtiofsd: Add TempFd structure

[Max Reitz]

We are planning to add file handles to lo_inode objects as an
alternative to lo_inode.fd.  That means that everywhere where we
currently reference lo_inode.fd, we will have to open a temporary file
descriptor that needs to be closed after use.

So instead of directly accessing lo_inode.fd, there will be a helper
function (lo_inode_fd()) that either returns lo_inode.fd, or opens a new
file descriptor with open_by_handle_at().  It encapsulates this result
in a TempFd structure to let the caller know whether the FD needs to be
closed after use (opened from the handle) or not (copied from
lo_inode.fd).

By using g_auto(TempFd) to store this result, callers will not even have
to care about closing a temporary FD after use.  It will be done
automatically once the object goes out of scope.

Signed-off-by: Max Reitz 
Reviewed-by: Connor Kuehl 
---
 tools/virtiofsd/passthrough_ll.c | 49 
 1 file changed, 49 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 1f27eeabc5..fb5e073e6a 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -178,6 +178,28 @@ struct lo_data {
 int user_posix_acl, posix_acl;
 };
 
+/**
+ * Represents a file descriptor that may either be owned by this
+ * TempFd, or only referenced (i.e. the ownership belongs to some
+ * other object, and the value has just been copied into this TempFd).
+ *
+ * The purpose of this encapsulation is to be used as g_auto(TempFd)
+ * to automatically clean up owned file descriptors when this object
+ * goes out of scope.
+ *
+ * Use temp_fd_steal() to get an owned file descriptor that will not
+ * be closed when the TempFd goes out of scope.
+ */
+typedef struct {
+int fd;
+bool owned; /* fd owned by this object? */
+} TempFd;
+
+#define TEMP_FD_INIT ((TempFd) { .fd = -1, .owned = false })
+
+static void temp_fd_clear(TempFd *temp_fd);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(TempFd, temp_fd_clear);
+
 static const struct fuse_opt lo_opts[] = {
 { "sandbox=namespace",
   offsetof(struct lo_data, sandbox),
@@ -255,6 +277,33 @@ static struct lo_data *lo_data(fuse_req_t req)
 return (struct lo_data *)fuse_req_userdata(req);
 }
 
+/**
+ * Clean-up function for TempFds
+ */
+static void temp_fd_clear(TempFd *temp_fd)
+{
+if (temp_fd->owned) {
+close(temp_fd->fd);
+*temp_fd = TEMP_FD_INIT;
+}
+}
+
+/**
+ * Return an owned fd from *temp_fd that will not be closed when
+ * *temp_fd goes out of scope.
+ *
+ * (TODO: Remove __attribute__ once this is used.)
+ */
+static __attribute__((unused)) int temp_fd_steal(TempFd *temp_fd)
+{
+if (temp_fd->owned) {
+temp_fd->owned = false;
+return temp_fd->fd;
+} else {
+return dup(temp_fd->fd);
+}
+}
+
 /*
  * Load capng's state from our saved state if the current thread
  * hadn't previously been loaded.
-- 
2.31.1

[PATCH v3 03/10] virtiofsd: Use lo_inode_open() instead of openat()

[Max Reitz]

The xattr functions want a non-O_PATH FD, so they reopen the lo_inode.fd
with the flags they need through /proc/self/fd.

Similarly, lo_opendir() needs an O_RDONLY FD.  Instead of the
/proc/self/fd trick, it just uses openat(fd, "."), because the FD is
guaranteed to be a directory, so this works.

All cases have one problem in common, though: In the future, when we may
have a file handle in the lo_inode instead of an FD, querying an
lo_inode FD may incur an open_by_handle_at() call.  It does not make
sense to then reopen that FD with custom flags, those should have been
passed to open_by_handle_at() instead.

Use lo_inode_open() instead of openat().  As part of the file handle
change, lo_inode_open() will be made to invoke openat() only if
lo_inode.fd is valid.  Otherwise, it will invoke open_by_handle_at()
with the right flags from the start.

Consequently, after this patch, lo_inode_open() is the only place to
invoke openat() to reopen an existing FD with different flags.

Signed-off-by: Max Reitz 
---
 tools/virtiofsd/passthrough_ll.c | 43 
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index fb5e073e6a..a444c3a7e2 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -1729,18 +1729,26 @@ static void lo_opendir(fuse_req_t req, fuse_ino_t ino,
 {
 int error = ENOMEM;
 struct lo_data *lo = lo_data(req);
-struct lo_dirp *d;
+struct lo_inode *inode;
+struct lo_dirp *d = NULL;
 int fd;
 ssize_t fh;
 
+inode = lo_inode(req, ino);
+if (!inode) {
+error = EBADF;
+goto out_err;
+}
+
 d = calloc(1, sizeof(struct lo_dirp));
 if (d == NULL) {
 goto out_err;
 }
 
-fd = openat(lo_fd(req, ino), ".", O_RDONLY);
-if (fd == -1) {
-goto out_errno;
+fd = lo_inode_open(lo, inode, O_RDONLY);
+if (fd < 0) {
+error = -fd;
+goto out_err;
 }
 
 d->dp = fdopendir(fd);
@@ -1769,6 +1777,7 @@ static void lo_opendir(fuse_req_t req, fuse_ino_t ino,
 out_errno:
 error = errno;
 out_err:
+lo_inode_put(lo, );
 if (d) {
 if (d->dp) {
 closedir(d->dp);
@@ -2973,7 +2982,6 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, 
const char *in_name,
 }
 }
 
-sprintf(procname, "%i", inode->fd);
 /*
  * It is not safe to open() non-regular/non-dir files in file server
  * unless O_PATH is used, so use that method for regular files/dir
@@ -2981,13 +2989,15 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, 
const char *in_name,
  * Otherwise, call fchdir() to avoid open().
  */
 if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
-fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+fd = lo_inode_open(lo, inode, O_RDONLY);
 if (fd < 0) {
-goto out_err;
+saverr = -fd;
+goto out;
 }
 ret = fgetxattr(fd, name, value, size);
 saverr = ret == -1 ? errno : 0;
 } else {
+sprintf(procname, "%i", inode->fd);
 /* fchdir should not fail here */
 FCHDIR_NOFAIL(lo->proc_self_fd);
 ret = getxattr(procname, name, value, size);
@@ -3054,15 +3064,16 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t 
ino, size_t size)
 }
 }
 
-sprintf(procname, "%i", inode->fd);
 if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
-fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+fd = lo_inode_open(lo, inode, O_RDONLY);
 if (fd < 0) {
-goto out_err;
+saverr = -fd;
+goto out;
 }
 ret = flistxattr(fd, value, size);
 saverr = ret == -1 ? errno : 0;
 } else {
+sprintf(procname, "%i", inode->fd);
 /* fchdir should not fail here */
 FCHDIR_NOFAIL(lo->proc_self_fd);
 ret = listxattr(procname, value, size);
@@ -3211,14 +3222,14 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, 
const char *in_name,
  * setxattr() on the link's filename there.
  */
 open_inode = S_ISREG(inode->filetype) || S_ISDIR(inode->filetype);
-sprintf(procname, "%i", inode->fd);
 if (open_inode) {
-fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+fd = lo_inode_open(lo, inode, O_RDONLY);
 if (fd < 0) {
-saverr = errno;
+saverr = -fd;
 goto out;
 }
 } else {
+sprintf(procname, "%i", inode->fd);
 /* fchdir should not fail here */
 FCHDIR_NOFAIL(lo->proc_self_fd);
 }
@@ -3317,16 +3328,16 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t 
ino, const char *in_name)
 fuse_log(FUSE_LOG_DEBUG, "lo_removexattr(ino=%" PRIu64 ", name=%s)\n", ino,
  name);
 
-sprintf(procname, "%i", inode->fd);
 if (S_ISREG(inode->filetype) || 

[PATCH v3 07/10] virtiofsd: Add lo_inode.fhandle

[Max Reitz]

This new field is an alternative to lo_inode.fd: Either of the two must
be set.  In case an O_PATH FD is needed for some lo_inode, it is either
taken from lo_inode.fd, if valid, or a temporary FD is opened with
open_by_handle_at().

Using a file handle instead of an FD has the advantage of keeping the
number of open file descriptors low.

Because open_by_handle_at() requires a mount FD (i.e. a non-O_PATH FD
opened on the filesystem to which the file handle refers), but every
lo_fhandle only has a mount ID (as returned by name_to_handle_at()), we
keep a hash map of such FDs in mount_fds (mapping ID to FD).
get_file_handle(), which is added by a later patch, will ensure that
every mount ID for which we have generated a handle has a corresponding
entry in mount_fds.

Signed-off-by: Max Reitz 
Reviewed-by: Connor Kuehl 
---
 tools/virtiofsd/passthrough_ll.c  | 116 ++
 tools/virtiofsd/passthrough_seccomp.c |   1 +
 2 files changed, 102 insertions(+), 15 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 292b7f7e27..487448d666 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -88,8 +88,25 @@ struct lo_key {
 uint64_t mnt_id;
 };
 
+struct lo_fhandle {
+union {
+struct file_handle handle;
+char padding[sizeof(struct file_handle) + MAX_HANDLE_SZ];
+};
+int mount_id;
+};
+
+/* Maps mount IDs to an FD that we can pass to open_by_handle_at() */
+static GHashTable *mount_fds;
+pthread_rwlock_t mount_fds_lock = PTHREAD_RWLOCK_INITIALIZER;
+
 struct lo_inode {
+/*
+ * Either of fd or fhandle must be set (i.e. >= 0 or non-NULL,
+ * respectively).
+ */
 int fd;
+struct lo_fhandle *fhandle;
 
 /*
  * Atomic reference count for this object.  The nlookup field holds a
@@ -302,6 +319,44 @@ static int temp_fd_steal(TempFd *temp_fd)
 }
 }
 
+/**
+ * Open the given file handle with the given flags.
+ *
+ * The mount FD to pass to open_by_handle_at() is taken from the
+ * mount_fds hash map.
+ *
+ * On error, return -errno.
+ */
+static int open_file_handle(const struct lo_fhandle *fh, int flags)
+{
+gpointer mount_fd_ptr;
+int mount_fd;
+bool found;
+int ret;
+
+ret = pthread_rwlock_rdlock(_fds_lock);
+if (ret) {
+return -ret;
+}
+
+/* mount_fd == 0 is valid, so we need lookup_extended */
+found = g_hash_table_lookup_extended(mount_fds,
+ GINT_TO_POINTER(fh->mount_id),
+ NULL, _fd_ptr);
+pthread_rwlock_unlock(_fds_lock);
+if (!found) {
+return -EINVAL;
+}
+mount_fd = GPOINTER_TO_INT(mount_fd_ptr);
+
+ret = open_by_handle_at(mount_fd, (struct file_handle *)>handle, 
flags);
+if (ret < 0) {
+return -errno;
+}
+
+return ret;
+}
+
 /*
  * Load capng's state from our saved state if the current thread
  * hadn't previously been loaded.
@@ -608,7 +663,11 @@ static void lo_inode_put(struct lo_data *lo, struct 
lo_inode **inodep)
 *inodep = NULL;
 
 if (g_atomic_int_dec_and_test(>refcount)) {
-close(inode->fd);
+if (inode->fd >= 0) {
+close(inode->fd);
+} else {
+g_free(inode->fhandle);
+}
 free(inode);
 }
 }
@@ -635,10 +694,25 @@ static struct lo_inode *lo_inode(fuse_req_t req, 
fuse_ino_t ino)
 
 static int lo_inode_fd(const struct lo_inode *inode, TempFd *tfd)
 {
-*tfd = (TempFd) {
-.fd = inode->fd,
-.owned = false,
-};
+if (inode->fd >= 0) {
+*tfd = (TempFd) {
+.fd = inode->fd,
+.owned = false,
+};
+} else {
+int fd;
+
+assert(inode->fhandle != NULL);
+fd = open_file_handle(inode->fhandle, O_PATH);
+if (fd < 0) {
+return -errno;
+}
+
+*tfd = (TempFd) {
+.fd = fd,
+.owned = true,
+};
+}
 
 return 0;
 }
@@ -678,22 +752,32 @@ static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd 
*tfd)
 static int lo_inode_open(const struct lo_data *lo, const struct lo_inode 
*inode,
  int open_flags, TempFd *tfd)
 {
-g_autofree char *fd_str = g_strdup_printf("%d", inode->fd);
+g_autofree char *fd_str = NULL;
 int fd;
 
 if (!S_ISREG(inode->filetype) && !S_ISDIR(inode->filetype)) {
 return -EBADF;
 }
 
-/*
- * The file is a symlink so O_NOFOLLOW must be ignored. We checked earlier
- * that the inode is not a special file but if an external process races
- * with us then symlinks are traversed here. It is not possible to escape
- * the shared directory since it is mounted as "/" though.
- */
-fd = openat(lo->proc_self_fd, fd_str, open_flags & ~O_NOFOLLOW);
-if (fd < 0) {
-return -errno;
+if (inode->fd >= 0) {
+/*
+ * The file is a symlink so 

[PATCH v3 00/10] virtiofsd: Allow using file handles instead of O_PATH FDs

[Max Reitz]

Hi,

v1 cover letter for an overview:
https://listman.redhat.com/archives/virtio-fs/2021-June/msg00033.html

v2 cover letter:
https://listman.redhat.com/archives/virtio-fs/2021-June/msg00074.html

For v3, at first I attempted to have errors related to file handle
generation (name_to_handle_at()) be returned to the guest unless they
are cases where file name generation is simply not supported, and only
then do a fallback to an O_PATH FD, as Vivek has suggested.

However, I found that to be rather complicated.  (Always falling back is
just simpler.)  Furthermore, because we believe that name_to_handle_at()
can rarely fail except for EOPNOTSUPP, there should be little difference
in practice.

Therefore, in v3, I kept the v2 model of always falling back to an
O_PATH FD when an error occurred during handle generation.

What did change in v3 is the following:
- I added patch 1, because f1aa1774dfb happened in the meantime, and
  this is basically what we did for virtiofsd-rs in the form of
  31e7ac63944 (virtiofsd-rs commit hash)

- Patch 4: In lookup_name(), I noticed that I failed to invoke
  lo_inode_put() to match the lo_inode() from the beginning of the
  function in all error paths.  Fixed by adding a common error path.

- Patch 6: Mostly contextual rebase conflicts (partly because of patch
  1), but also one functional change: I Dropped the `assert(fd >= 0)`
  under `if (open_inode)` in lo_setxattr(), because `fd` is dropped by
  this patch (and `inode_fd` is used regardless of the value of
  `open_inode` we can’t assert anything similar on it).

- Patch 8:
  - Fixed the condition to reject results found by st_ino lookup.
- st_ino on its own is only a valid identifier/key if we have an
  O_PATH fd for its respective lo_inode, because otherwise the inode
  may be unlinked and its st_ino might be reused by some new inode
- It does not matter whether lo_find()’s caller has supplied a file
  handle for a prior lookup by handle or not, so drop that part of
  the condition
- Semantically, it does not matter whether the lo_inode has a file
  handle or not – what matters is whether it has an O_PATH fd or
  not.  (The two are linked by a `handle <=> !fd` condition, so that
  part wasn’t technically wrong, just semantically.)
- In accordance with the last point, I rewrote the comment
  explaining why we have to reject such results.
  - Rebase conflict in lookup_name() because of the fix in patch 4

- Patch 9:
  - Non-functional change in lo_do_lookup() to separate the
get_file_handle()/openat() part from the do_statx() calls (and have
the do_statx() calls be side by side) – as a side effect, this makes
the diff to master slightly smaller.
  - Rebase conflict in lookup_name() because of the fix in patch 4

- Patch 10:
  - Rebase conflict in lookup_name() because of the fix in patch 4


Max Reitz (10):
  virtiofsd: Limit setxattr()'s creds-dropped region
  virtiofsd: Add TempFd structure
  virtiofsd: Use lo_inode_open() instead of openat()
  virtiofsd: Add lo_inode_fd() helper
  virtiofsd: Let lo_fd() return a TempFd
  virtiofsd: Let lo_inode_open() return a TempFd
  virtiofsd: Add lo_inode.fhandle
  virtiofsd: Add inodes_by_handle hash table
  virtiofsd: Optionally fill lo_inode.fhandle
  virtiofsd: Add lazy lo_do_find()

 tools/virtiofsd/helper.c  |   3 +
 tools/virtiofsd/passthrough_ll.c  | 869 +-
 tools/virtiofsd/passthrough_seccomp.c |   2 +
 3 files changed, 720 insertions(+), 154 deletions(-)

-- 
2.31.1

[PATCH v3 08/10] virtiofsd: Add inodes_by_handle hash table

[Max Reitz]

Currently, lo_inode.fhandle is always NULL and so always keep an O_PATH
FD in lo_inode.fd.  Therefore, when the respective inode is unlinked,
its inode ID will remain in use until we drop our lo_inode (and
lo_inode_put() thus closes the FD).  Therefore, lo_find() can safely use
the inode ID as an lo_inode key, because any inode with an inode ID we
find in lo_data.inodes (on the same filesystem) must be the exact same
file.

This will change when we start setting lo_inode.fhandle so we do not
have to keep an O_PATH FD open.  Then, unlinking such an inode will
immediately remove it, so its ID can then be reused by newly created
files, even while the lo_inode object is still there[1].

So creating a new file can then reuse the old file's inode ID, and
looking up the new file would lead to us finding the old file's
lo_inode, which is not ideal.

Luckily, just as file handles cause this problem, they also solve it:  A
file handle contains a generation ID, which changes when an inode ID is
reused, so the new file can be distinguished from the old one.  So all
we need to do is to add a second map besides lo_data.inodes that maps
file handles to lo_inodes, namely lo_data.inodes_by_handle.  For
clarity, lo_data.inodes is renamed to lo_data.inodes_by_ids.

Unfortunately, we cannot rely on being able to generate file handles
every time.  Therefore, we still enter every lo_inode object into
inodes_by_ids, but having an entry in inodes_by_handle is optional.  A
potential inodes_by_handle entry then has precedence, the inodes_by_ids
entry is just a fallback.

Note that we do not generate lo_fhandle objects yet, and so we also do
not enter anything into the inodes_by_handle map yet.  Also, all lookups
skip that map.  We might manually create file handles with some code
that is immediately removed by the next patch again, but that would
break the assumption in lo_find() that every lo_inode with a non-NULL
.fhandle must have an entry in inodes_by_handle and vice versa.  So we
leave actually using the inodes_by_handle map for the next patch.

[1] If some application in the guest still has the file open, there is
going to be a corresponding FD mapping in lo_data.fd_map.  In such a
case, the inode will only go away once every application in the guest
has closed it.  The problem described only applies to cases where the
guest does not have the file open, and it is just in the dentry cache,
basically.

Signed-off-by: Max Reitz 
---
 tools/virtiofsd/passthrough_ll.c | 81 +---
 1 file changed, 65 insertions(+), 16 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 487448d666..f9d8b2f134 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -180,7 +180,8 @@ struct lo_data {
 int announce_submounts;
 bool use_statx;
 struct lo_inode root;
-GHashTable *inodes; /* protected by lo->mutex */
+GHashTable *inodes_by_ids; /* protected by lo->mutex */
+GHashTable *inodes_by_handle; /* protected by lo->mutex */
 struct lo_map ino_map; /* protected by lo->mutex */
 struct lo_map dirp_map; /* protected by lo->mutex */
 struct lo_map fd_map; /* protected by lo->mutex */
@@ -263,8 +264,9 @@ static struct {
 /* That we loaded cap-ng in the current thread from the saved */
 static __thread bool cap_loaded = 0;
 
-static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st,
-uint64_t mnt_id);
+static struct lo_inode *lo_find(struct lo_data *lo,
+const struct lo_fhandle *fhandle,
+struct stat *st, uint64_t mnt_id);
 static int xattr_map_client(const struct lo_data *lo, const char *client_name,
 char **out_name);
 
@@ -1064,18 +1066,40 @@ out_err:
 fuse_reply_err(req, saverr);
 }
 
-static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st,
-uint64_t mnt_id)
+static struct lo_inode *lo_find(struct lo_data *lo,
+const struct lo_fhandle *fhandle,
+struct stat *st, uint64_t mnt_id)
 {
-struct lo_inode *p;
-struct lo_key key = {
+struct lo_inode *p = NULL;
+struct lo_key ids_key = {
 .ino = st->st_ino,
 .dev = st->st_dev,
 .mnt_id = mnt_id,
 };
 
 pthread_mutex_lock(>mutex);
-p = g_hash_table_lookup(lo->inodes, );
+if (fhandle) {
+p = g_hash_table_lookup(lo->inodes_by_handle, fhandle);
+}
+if (!p) {
+p = g_hash_table_lookup(lo->inodes_by_ids, _key);
+/*
+ * When we had to fall back to looking up an inode by its
+ * inode ID, ensure that we hit an entry that has a valid file
+ * descriptor.  Having an FD open means that the inode cannot
+ * really be deleted until the FD is closed, so that the inode
+ * ID remains valid until we evict our 

[PATCH v3 05/10] virtiofsd: Let lo_fd() return a TempFd

[Max Reitz]

Accessing lo_inode.fd must generally happen through lo_inode_fd(), and
lo_fd() is no exception; and then it must pass on the TempFd it has
received from lo_inode_fd().

(Note that all lo_fd() calls now use proper error handling, where all of
them were in-line before; i.e. they were used in place of the fd
argument of some function call.  This only worked because the only error
that could occur was that lo_inode() failed to find the inode ID: Then
-1 would be passed as the fd, which would result in an EBADF error,
which is precisely what we would want to return to the guest for an
invalid inode ID.
Now, though, lo_inode_fd() might potentially invoke open_by_handle_at(),
which can return many different errors, and they should be properly
handled and returned to the guest.  So we can no longer allow lo_fd() to
be used in-line, and instead need to do proper error handling for it.)

Signed-off-by: Max Reitz 
Reviewed-by: Connor Kuehl 
---
 tools/virtiofsd/passthrough_ll.c | 55 +---
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 86b901cf19..9e1bc37af8 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -650,18 +650,19 @@ static int lo_inode_fd(const struct lo_inode *inode, 
TempFd *tfd)
  * they are done with the fd.  This will be done in a later patch to make
  * review easier.
  */
-static int lo_fd(fuse_req_t req, fuse_ino_t ino)
+static int lo_fd(fuse_req_t req, fuse_ino_t ino, TempFd *tfd)
 {
 struct lo_inode *inode = lo_inode(req, ino);
-int fd;
+int res;
 
 if (!inode) {
-return -1;
+return -EBADF;
 }
 
-fd = inode->fd;
+res = lo_inode_fd(inode, tfd);
+
 lo_inode_put(lo_data(req), );
-return fd;
+return res;
 }
 
 /*
@@ -798,14 +799,19 @@ static void lo_init(void *userdata, struct fuse_conn_info 
*conn)
 static void lo_getattr(fuse_req_t req, fuse_ino_t ino,
struct fuse_file_info *fi)
 {
+g_auto(TempFd) ino_fd = TEMP_FD_INIT;
 int res;
 struct stat buf;
 struct lo_data *lo = lo_data(req);
 
 (void)fi;
 
-res =
-fstatat(lo_fd(req, ino), "", , AT_EMPTY_PATH | 
AT_SYMLINK_NOFOLLOW);
+res = lo_fd(req, ino, _fd);
+if (res < 0) {
+return (void)fuse_reply_err(req, -res);
+}
+
+res = fstatat(ino_fd.fd, "", , AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
 if (res == -1) {
 return (void)fuse_reply_err(req, errno);
 }
@@ -1529,6 +1535,7 @@ out:
 
 static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name)
 {
+g_auto(TempFd) parent_fd = TEMP_FD_INIT;
 int res;
 struct lo_inode *inode;
 struct lo_data *lo = lo_data(req);
@@ -1543,13 +1550,19 @@ static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, 
const char *name)
 return;
 }
 
+res = lo_fd(req, parent, _fd);
+if (res < 0) {
+fuse_reply_err(req, -res);
+return;
+}
+
 inode = lookup_name(req, parent, name);
 if (!inode) {
 fuse_reply_err(req, EIO);
 return;
 }
 
-res = unlinkat(lo_fd(req, parent), name, AT_REMOVEDIR);
+res = unlinkat(parent_fd.fd, name, AT_REMOVEDIR);
 
 fuse_reply_err(req, res == -1 ? errno : 0);
 unref_inode_lolocked(lo, inode, 1);
@@ -1635,6 +1648,7 @@ out:
 
 static void lo_unlink(fuse_req_t req, fuse_ino_t parent, const char *name)
 {
+g_auto(TempFd) parent_fd = TEMP_FD_INIT;
 int res;
 struct lo_inode *inode;
 struct lo_data *lo = lo_data(req);
@@ -1649,13 +1663,19 @@ static void lo_unlink(fuse_req_t req, fuse_ino_t 
parent, const char *name)
 return;
 }
 
+res = lo_fd(req, parent, _fd);
+if (res < 0) {
+fuse_reply_err(req, -res);
+return;
+}
+
 inode = lookup_name(req, parent, name);
 if (!inode) {
 fuse_reply_err(req, EIO);
 return;
 }
 
-res = unlinkat(lo_fd(req, parent), name, 0);
+res = unlinkat(parent_fd.fd, name, 0);
 
 fuse_reply_err(req, res == -1 ? errno : 0);
 unref_inode_lolocked(lo, inode, 1);
@@ -1735,10 +1755,16 @@ static void lo_forget_multi(fuse_req_t req, size_t 
count,
 
 static void lo_readlink(fuse_req_t req, fuse_ino_t ino)
 {
+g_auto(TempFd) ino_fd = TEMP_FD_INIT;
 char buf[PATH_MAX + 1];
 int res;
 
-res = readlinkat(lo_fd(req, ino), "", buf, sizeof(buf));
+res = lo_fd(req, ino, _fd);
+if (res < 0) {
+return (void)fuse_reply_err(req, -res);
+}
+
+res = readlinkat(ino_fd.fd, "", buf, sizeof(buf));
 if (res == -1) {
 return (void)fuse_reply_err(req, errno);
 }
@@ -2535,10 +2561,17 @@ static void lo_write_buf(fuse_req_t req, fuse_ino_t ino,
 
 static void lo_statfs(fuse_req_t req, fuse_ino_t ino)
 {
+g_auto(TempFd) ino_fd = TEMP_FD_INIT;
 int res;
 struct statvfs stbuf;
 
-res = fstatvfs(lo_fd(req, ino), );
+res = lo_fd(req, ino, _fd);
+

[PATCH v3 01/10] virtiofsd: Limit setxattr()'s creds-dropped region

[Max Reitz]

We only need to drop/switch our credentials for the (f)setxattr() call
alone, not for the openat() or fchdir() around it.

(Right now, this may not be that big of a problem, but with inodes being
identified by file handles instead of an O_PATH fd, we will need
open_by_handle_at() calls here, which is really fickle when it comes to
credentials being dropped.)

Signed-off-by: Max Reitz 
---
 tools/virtiofsd/passthrough_ll.c | 34 +++-
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 38b2af8599..1f27eeabc5 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3121,6 +3121,7 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, 
const char *in_name,
 bool switched_creds = false;
 bool cap_fsetid_dropped = false;
 struct lo_cred old = {};
+bool open_inode;
 
 if (block_xattr(lo, in_name)) {
 fuse_reply_err(req, EOPNOTSUPP);
@@ -3155,7 +3156,24 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, 
const char *in_name,
 fuse_log(FUSE_LOG_DEBUG, "lo_setxattr(ino=%" PRIu64
  ", name=%s value=%s size=%zd)\n", ino, name, value, size);
 
+/*
+ * We can only open regular files or directories.  If the inode is
+ * something else, we have to enter /proc/self/fd and use
+ * setxattr() on the link's filename there.
+ */
+open_inode = S_ISREG(inode->filetype) || S_ISDIR(inode->filetype);
 sprintf(procname, "%i", inode->fd);
+if (open_inode) {
+fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+if (fd < 0) {
+saverr = errno;
+goto out;
+}
+} else {
+/* fchdir should not fail here */
+FCHDIR_NOFAIL(lo->proc_self_fd);
+}
+
 /*
  * If we are setting posix access acl and if SGID needs to be
  * cleared, then switch to caller's gid and drop CAP_FSETID
@@ -3176,20 +3194,13 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, 
const char *in_name,
 }
 switched_creds = true;
 }
-if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
-fd = openat(lo->proc_self_fd, procname, O_RDONLY);
-if (fd < 0) {
-saverr = errno;
-goto out;
-}
+if (open_inode) {
+assert(fd >= 0);
 ret = fsetxattr(fd, name, value, size, flags);
 saverr = ret == -1 ? errno : 0;
 } else {
-/* fchdir should not fail here */
-FCHDIR_NOFAIL(lo->proc_self_fd);
 ret = setxattr(procname, name, value, size, flags);
 saverr = ret == -1 ? errno : 0;
-FCHDIR_NOFAIL(lo->root.fd);
 }
 if (switched_creds) {
 if (cap_fsetid_dropped)
@@ -3198,6 +3209,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, 
const char *in_name,
 lo_restore_cred(, false);
 }
 
+if (!open_inode) {
+/* Change CWD back, fchdir should not fail here */
+FCHDIR_NOFAIL(lo->root.fd);
+}
+
 out:
 if (fd >= 0) {
 close(fd);
-- 
2.31.1

[PATCH v3 04/10] virtiofsd: Add lo_inode_fd() helper

[Max Reitz]

Once we let lo_inode.fd be optional, we will need its users to open the
file handle stored in lo_inode instead.  This function will do that.

For now, it just returns lo_inode.fd, though.

Signed-off-by: Max Reitz 
---
 tools/virtiofsd/passthrough_ll.c | 150 +--
 1 file changed, 125 insertions(+), 25 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index a444c3a7e2..86b901cf19 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -635,6 +635,16 @@ static struct lo_inode *lo_inode(fuse_req_t req, 
fuse_ino_t ino)
 return elem->inode;
 }
 
+static int lo_inode_fd(const struct lo_inode *inode, TempFd *tfd)
+{
+*tfd = (TempFd) {
+.fd = inode->fd,
+.owned = false,
+};
+
+return 0;
+}
+
 /*
  * TODO Remove this helper and force callers to hold an inode refcount until
  * they are done with the fd.  This will be done in a later patch to make
@@ -822,11 +832,11 @@ static int lo_fi_fd(fuse_req_t req, struct fuse_file_info 
*fi)
 static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
int valid, struct fuse_file_info *fi)
 {
+g_auto(TempFd) inode_fd = TEMP_FD_INIT;
 int saverr;
 char procname[64];
 struct lo_data *lo = lo_data(req);
 struct lo_inode *inode;
-int ifd;
 int res;
 int fd = -1;
 
@@ -836,7 +846,11 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 return;
 }
 
-ifd = inode->fd;
+res = lo_inode_fd(inode, _fd);
+if (res < 0) {
+saverr = -res;
+goto out_err;
+}
 
 /* If fi->fh is invalid we'll report EBADF later */
 if (fi) {
@@ -847,7 +861,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 if (fi) {
 res = fchmod(fd, attr->st_mode);
 } else {
-sprintf(procname, "%i", ifd);
+sprintf(procname, "%i", inode_fd.fd);
 res = fchmodat(lo->proc_self_fd, procname, attr->st_mode, 0);
 }
 if (res == -1) {
@@ -859,12 +873,13 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 uid_t uid = (valid & FUSE_SET_ATTR_UID) ? attr->st_uid : (uid_t)-1;
 gid_t gid = (valid & FUSE_SET_ATTR_GID) ? attr->st_gid : (gid_t)-1;
 
-saverr = drop_security_capability(lo, ifd);
+saverr = drop_security_capability(lo, inode_fd.fd);
 if (saverr) {
 goto out_err;
 }
 
-res = fchownat(ifd, "", uid, gid, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
+res = fchownat(inode_fd.fd, "", uid, gid,
+   AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
 if (res == -1) {
 saverr = errno;
 goto out_err;
@@ -943,7 +958,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, 
struct stat *attr,
 if (fi) {
 res = futimens(fd, tv);
 } else {
-sprintf(procname, "%i", inode->fd);
+sprintf(procname, "%i", inode_fd.fd);
 res = utimensat(lo->proc_self_fd, procname, tv, 0);
 }
 if (res == -1) {
@@ -1058,7 +1073,8 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t 
parent, const char *name,
 struct fuse_entry_param *e,
 struct lo_inode **inodep)
 {
-int newfd;
+g_auto(TempFd) dir_fd = TEMP_FD_INIT;
+int newfd = -1;
 int res;
 int saverr;
 uint64_t mnt_id;
@@ -1088,7 +1104,13 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t 
parent, const char *name,
 name = ".";
 }
 
-newfd = openat(dir->fd, name, O_PATH | O_NOFOLLOW);
+res = lo_inode_fd(dir, _fd);
+if (res < 0) {
+saverr = -res;
+goto out;
+}
+
+newfd = openat(dir_fd.fd, name, O_PATH | O_NOFOLLOW);
 if (newfd == -1) {
 goto out_err;
 }
@@ -1155,6 +1177,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t 
parent, const char *name,
 
 out_err:
 saverr = errno;
+out:
 if (newfd != -1) {
 close(newfd);
 }
@@ -1312,6 +1335,7 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t 
parent,
  const char *name, mode_t mode, dev_t rdev,
  const char *link)
 {
+g_auto(TempFd) dir_fd = TEMP_FD_INIT;
 int res;
 int saverr;
 struct lo_data *lo = lo_data(req);
@@ -1335,12 +1359,18 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t 
parent,
 return;
 }
 
+res = lo_inode_fd(dir, _fd);
+if (res < 0) {
+saverr = -res;
+goto out;
+}
+
 saverr = lo_change_cred(req, , lo->change_umask && !S_ISLNK(mode));
 if (saverr) {
 goto out;
 }
 
-res = mknod_wrapper(dir->fd, name, link, mode, rdev);
+res = mknod_wrapper(dir_fd.fd, name, link, mode, rdev);
 
 saverr = errno;
 
@@ -1388,6 +1418,8 @@ static void 

Re: [PATCH 3/3] gitlab-ci: Fix ..._RUNNER_AVAILABLE variables and document them

[Philippe Mathieu-Daudé]

On 7/30/21 4:38 PM, Thomas Huth wrote:
> The patch that recently introduced the S390X_RUNNER_AVAILABLE variable
> in custom-runners.yml missed that the bottom half of the file is rather
> about aarch64 than s390x. Thus rename the S390X_RUNNER_AVAILABLE to
> AARCH64_RUNNER_AVAILABLE in those jobs.

Oops.

> Finally mention both variables in our CI documentation, too.

Thanks.

Maybe "Fix ${arch}_RUNNER_AVAILABLE" as subject.

Reviewed-by: Philippe Mathieu-Daudé 

> Fixes: c5dd0f0342 ("Improve rules for the staging branch")
> Signed-off-by: Thomas Huth 
> ---
>  .gitlab-ci.d/custom-runners.yml | 12 ++--
>  docs/devel/ci.rst   | 13 +
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
> index 564b94565d..0d3e4a7b4b 100644
> --- a/.gitlab-ci.d/custom-runners.yml
> +++ b/.gitlab-ci.d/custom-runners.yml
> @@ -137,7 +137,7 @@ ubuntu-20.04-aarch64-all-linux-static:
>   - aarch64
>   rules:
>   - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
> /^staging/'
> - - if: "$S390X_RUNNER_AVAILABLE"
> + - if: "$AARCH64_RUNNER_AVAILABLE"
>   script:
>   # --disable-libssh is needed because of 
> https://bugs.launchpad.net/qemu/+bug/1838763
>   # --disable-glusterfs is needed because there's no static version of those 
> libs in distro supplied packages
> @@ -157,7 +157,7 @@ ubuntu-20.04-aarch64-all:
>   - aarch64
>   rules:
>   - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
> /^staging/'
> - - if: "$S390X_RUNNER_AVAILABLE"
> + - if: "$AARCH64_RUNNER_AVAILABLE"
>   script:
>   - mkdir build
>   - cd build
> @@ -174,7 +174,7 @@ ubuntu-20.04-aarch64-alldbg:
>   - aarch64
>   rules:
>   - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
> /^staging/'
> - - if: "$S390X_RUNNER_AVAILABLE"
> + - if: "$AARCH64_RUNNER_AVAILABLE"
>   script:
>   - mkdir build
>   - cd build
> @@ -193,7 +193,7 @@ ubuntu-20.04-aarch64-clang:
>   rules:
>   - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
> /^staging/'
> when: manual
> - - if: "$S390X_RUNNER_AVAILABLE"
> + - if: "$AARCH64_RUNNER_AVAILABLE"
> when: manual
>   script:
>   - mkdir build
> @@ -211,7 +211,7 @@ ubuntu-20.04-aarch64-tci:
>   - aarch64
>   rules:
>   - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
> /^staging/'
> - - if: "$S390X_RUNNER_AVAILABLE"
> + - if: "$AARCH64_RUNNER_AVAILABLE"
>   script:
>   - mkdir build
>   - cd build
> @@ -228,7 +228,7 @@ ubuntu-20.04-aarch64-notcg:
>   rules:
>   - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
> /^staging/'
> when: manual
> - - if: "$S390X_RUNNER_AVAILABLE"
> + - if: "$AARCH64_RUNNER_AVAILABLE"
> when: manual
>   script:
>   - mkdir build
> diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst
> index 205572510c..558327457c 100644
> --- a/docs/devel/ci.rst
> +++ b/docs/devel/ci.rst
> @@ -48,6 +48,19 @@ these artifacts are not already cached, downloading them 
> make the jobs
>  reach the timeout limit). Set this variable to have the tests using the
>  Avocado framework run automatically.
>  
> +AARCH64_RUNNER_AVAILABLE
> +
> +If you've got access to an aarch64 host that can be used as a gitlab-CI
> +runner, you can set this variable to enable the tests that require this
> +kind of host. The runner should be tagged with "aarch64".
> +
> +S390X_RUNNER_AVAILABLE
> +~~
> +If you've got access to an IBM Z host that can be used as a gitlab-CI
> +runner, you can set this variable to enable the tests that require this
> +kind of host. The runner should be tagged with "s390x".
> +
> +
>  Jobs on Custom Runners
>  ==
>  
> 

Re: [PATCH v3 0/2] x86/sev: Measured Linux SEV guest with kernel/initrd/cmdline

[Connor Kuehl]

On Thu Jul 29, 2021 at 2:31 PM CDT, Dov Murik wrote:
> The OVMF companion series has been reviewed by the new OVMF maintainer
> and merged to edk2 master branch as of edk2 commit 514b3aa08ece [1].
>
> [1] https://github.com/tianocore/edk2/commit/514b3aa08ece

Awesome! Unfortunately, it's looking like we'll have to wait[1] for QEMU to
thaw before this series goes in.

Connor

[1] https://wiki.qemu.org/Planning/6.1

[PATCH 3/3] gitlab-ci: Fix ..._RUNNER_AVAILABLE variables and document them

[Thomas Huth]

The patch that recently introduced the S390X_RUNNER_AVAILABLE variable
in custom-runners.yml missed that the bottom half of the file is rather
about aarch64 than s390x. Thus rename the S390X_RUNNER_AVAILABLE to
AARCH64_RUNNER_AVAILABLE in those jobs.

Finally mention both variables in our CI documentation, too.

Fixes: c5dd0f0342 ("Improve rules for the staging branch")
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/custom-runners.yml | 12 ++--
 docs/devel/ci.rst   | 13 +
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
index 564b94565d..0d3e4a7b4b 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -137,7 +137,7 @@ ubuntu-20.04-aarch64-all-linux-static:
  - aarch64
  rules:
  - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
/^staging/'
- - if: "$S390X_RUNNER_AVAILABLE"
+ - if: "$AARCH64_RUNNER_AVAILABLE"
  script:
  # --disable-libssh is needed because of 
https://bugs.launchpad.net/qemu/+bug/1838763
  # --disable-glusterfs is needed because there's no static version of those 
libs in distro supplied packages
@@ -157,7 +157,7 @@ ubuntu-20.04-aarch64-all:
  - aarch64
  rules:
  - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
/^staging/'
- - if: "$S390X_RUNNER_AVAILABLE"
+ - if: "$AARCH64_RUNNER_AVAILABLE"
  script:
  - mkdir build
  - cd build
@@ -174,7 +174,7 @@ ubuntu-20.04-aarch64-alldbg:
  - aarch64
  rules:
  - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
/^staging/'
- - if: "$S390X_RUNNER_AVAILABLE"
+ - if: "$AARCH64_RUNNER_AVAILABLE"
  script:
  - mkdir build
  - cd build
@@ -193,7 +193,7 @@ ubuntu-20.04-aarch64-clang:
  rules:
  - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
/^staging/'
when: manual
- - if: "$S390X_RUNNER_AVAILABLE"
+ - if: "$AARCH64_RUNNER_AVAILABLE"
when: manual
  script:
  - mkdir build
@@ -211,7 +211,7 @@ ubuntu-20.04-aarch64-tci:
  - aarch64
  rules:
  - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
/^staging/'
- - if: "$S390X_RUNNER_AVAILABLE"
+ - if: "$AARCH64_RUNNER_AVAILABLE"
  script:
  - mkdir build
  - cd build
@@ -228,7 +228,7 @@ ubuntu-20.04-aarch64-notcg:
  rules:
  - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ 
/^staging/'
when: manual
- - if: "$S390X_RUNNER_AVAILABLE"
+ - if: "$AARCH64_RUNNER_AVAILABLE"
when: manual
  script:
  - mkdir build
diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst
index 205572510c..558327457c 100644
--- a/docs/devel/ci.rst
+++ b/docs/devel/ci.rst
@@ -48,6 +48,19 @@ these artifacts are not already cached, downloading them 
make the jobs
 reach the timeout limit). Set this variable to have the tests using the
 Avocado framework run automatically.
 
+AARCH64_RUNNER_AVAILABLE
+
+If you've got access to an aarch64 host that can be used as a gitlab-CI
+runner, you can set this variable to enable the tests that require this
+kind of host. The runner should be tagged with "aarch64".
+
+S390X_RUNNER_AVAILABLE
+~~
+If you've got access to an IBM Z host that can be used as a gitlab-CI
+runner, you can set this variable to enable the tests that require this
+kind of host. The runner should be tagged with "s390x".
+
+
 Jobs on Custom Runners
 ==
 
-- 
2.27.0

[PATCH 0/3] Gitlab-CI improvements

[Thomas Huth]

Here are three patches for some small issues that I noticed in our
gitlab-CI files recently...

Thomas Huth (3):
  gitlab-ci: Merge "build-disabled" with "build-without-default-features"
  gitlab-ci: Remove superfluous "dnf install" statement
  gitlab-ci: Fix ..._RUNNER_AVAILABLE variables and document them

 .gitlab-ci.d/buildtest.yml  | 99 +
 .gitlab-ci.d/custom-runners.yml | 12 ++--
 docs/devel/ci.rst   | 13 +
 3 files changed, 32 insertions(+), 92 deletions(-)

-- 
2.27.0

[PATCH 2/3] gitlab-ci: Remove superfluous "dnf install" statement

[Thomas Huth]

The container already features meson and ninja, so there is no need
to try to install it with dnf again.

Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/buildtest.yml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index f390f98044..38f08452f1 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -590,8 +590,6 @@ build-libvhost-user:
   image: $CI_REGISTRY_IMAGE/qemu/fedora:latest
   needs:
 job: amd64-fedora-container
-  before_script:
-- dnf install -y meson ninja-build
   script:
 - mkdir subprojects/libvhost-user/build
 - cd subprojects/libvhost-user/build
-- 
2.27.0