[xen-4.15-testing test] 163710: tolerable FAIL - PUSHED

[osstest service owner]

flight 163710 xen-4.15-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/163710/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 162882
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 162882
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 162882
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 162882
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 162882
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 162882
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 162882
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 162882
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 162882
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 162882
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 162882
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  1a6824957d051bb94929a1c74a52c013bc2f388d
baseline version:
 xen  ec457ac2a29279e8cd91745c410b0f49d5e8f1ff

Last test of basis   162882  2021-06-17 19:07:29 Z   28 days
Testing same since   163710  2021-07-15 07:36:48 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  Dario Faggioli 
  George Dunlap 
  Jan Beulich 
  Juergen Gross 
  Tamas K Lengyel 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   

Suggested changes to the admission policy of the vulnerability pre-disclosure list

[Charles-H. Schulz]

Hello,

I /we /Vates would like to suggest some changes to the policy regarding the
enrollment to the pre-disclosure mailing list of the Xen Security Team.

We have had some talks with the French national CERT who has a need to be the
recipient of such a list. This national CERT -and in my experience other
national CERTs such as the NIST for instance- is in constant contact with a
large Xen userbase that is mostly made up of large parts of the public sector
as well as critical infrastructure operators belonging to the private
sector. For confidentiality reasons they cannot disclose who uses Xen and
where it is used nor who may be using it internally or within the related
national cybersecurity authority.

Because of that, their request may not be clear or matching the existing
criteria for inclusion in the mailing list. National CERTs are trusted
actors and have historically been among the very first entities to define,
advocate for and put in practice the very notion of responsible
disclosure. Much of the current practice of Open Source projects in that
regard actually stems from CERTs. As part of their policies and processes
regarding vulnerability disclosure, the notion of confidentiality and
documented, waterfall-like processes of disclosure is play an integral
part of
how they handle informaton and publicity around vulnerability. As a result,
national CERTs (and the French National CERT) do not spread undisclosed
vulnerability without following established and agreed-upon processes. Such
processes include, in our instance, the ones defined and followed by the Xen
Security Team. Compliance with these are the first criteria to earn trust and
respect from the ecosystem and the downstream users. You can see an example
of their work here: https://www.cert.ssi.gouv.fr/

Part of the mission of the French National CERT is to work with
critical infrastructure providers in securing their IT.
This kind of expertise entails the securing of these information
systems before any unforeseen incident as well as after the incident
(incident remediation).
None of the tasks involved imply the communication of zero-day types
of vulnerabilities or vulnerabilities that are unpublished to the
downstream users.

I hope this clarifies the request and I'm looking forward to your feedback.

Best regards,

-- 
Charles-H. Schulz
Chief Strategy Officer - CSO
XCP-ng & Xen Orchestra - Vates solutions



signature.asc
Description: PGP signature

[xen-4.14-testing test] 163709: tolerable FAIL - PUSHED

[osstest service owner]

flight 163709 xen-4.14-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/163709/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 162891
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 162891
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 162891
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 162891
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 162891
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 162891
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 162891
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 162891
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 162891
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 162891
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 162891
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass

version targeted for testing:
 xen  3cfccd70992b3d2b739e3febfceb99fdf6283be0
baseline version:
 xen  b6a8c4f72def4d1135ff42660a86276ce2565c8c

Last test of basis   162891  2021-06-18 11:05:12 Z   27 days
Testing same since   163709  2021-07-15 07:36:47 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  Dario Faggioli 
  George Dunlap 
  Jan Beulich 
  Tamas K Lengyel 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm

Re: [PATCH v2 0/4] Remove unconditional arch dependency on asm/debugger.h

[Andrew Cooper]

On 14/07/2021 21:37, Bobby Eshleman wrote:
> Currently, any architecture wishing to use common/ is likely
> to be required to implement the functions found in "asm/debugger.h".
> Some architectures, however, do not have an actual use for these
> functions and so are forced to implement stubs.  This patch does the
> following:
>
> * Supplies common stubs if !CONFIG_CRASH_DEBUG for any architecture,
>   removing the need for all new architectures to have "asm/debugger.h".
> * Moves parts of the x86 implementation to "arch/x86/debugger.c".
> * Removes the ARM calls to its stubs.
> * Centralizes non-inlined x86 code conditionally compiled by 
> CONFIG_CRASH_DEBUG
>   into arch/x86/debugger.c, which is now conditionally built for
>   CONFIG_CRASH_DEBUG via Kbuild (i.e., obj-$(CONFIG_CRASH_DEBUG)).
> * Tries to improve the x86 implementation by not inlining large
>   functions (but preserving inlining for those that seemed "small").

My replies from yesterday appear to have got lost.  Lets try it again. 
Jan already picked up on the header file and commit change in patch 1.

However, patch 2 actually demonstrates a massive confusion which exists
in the x86 code.  We have two things called debugger, which are
unrelated, but mixed in asm-x86/debugger.h

There is gdbstub itself, which is an implementation of the gdb remote
debugging protocol over serial.  (I've never seen anyone use this in a
decade, and the logic isn't remotely SMP-safe at all, so I'm very
tempted to suggest ripping it out completely, but lets ignore that for now).

Then we have debugger_trap_*() which claims to be arch-neutral wrappers
to a common debugging interface, which is only actually backed by
gdbstub in x86.  Both of these facilities are to do with debugging Xen
itself when Xen crashes.


Then there is gdbsx which is totally unrelated to the above, and is a
daemon in dom0 to translate the gdb remote protocol into a set of
hypercalls to perform on a guest under test. 
domain_pause_for_debugger() is gdbsx functionality, and nothing to do
with Xen crashing.

On top of that, debugger_trap_entry() is actually a layering violation
merging the two.

Therefore, I recommend the following, in this order:

1) Patch emptying debugger_trap_entry() and expanding the contents
inline in do_int3/debug().  Both already have an if ( !guest_mode() )
path, so add an else if ( ... ) clause.  This supersedes patch 3. 
(Also, fix the logic to have "const struct vcpu *curr = current" and
avoid the opencoded use of current lower down).

curr->arch.gdbsx_vcpu_event only being set for TRAP_int3 looks totally
bogus (the non-int3 paths cause gdbsx to miss notifications), but is
repeated all across Xen.  Keep the logic unchanged across the move, and
leave fixing gdbsx bugs to some future point.

2) Patch (or patches) renaming arch/x86/debug.c to arch/x86/gdbsx.c, and
add a new include/asm-x86/gdbsx.h.

domain_pause_for_debugger() wants moving (prototype and definition)
which subsumes patch 4, and deletes domain.c's include of debugger.h

domctl.s ifdef'd gdbsx_guest_mem_io() wants moving too, as it has one
caller, and is the sole caller of dbg_rw_mem().  The two functions
likely want merging so we don't just have a wrapper making trivial API
change.  This will also require some header file renames.

With this done, there is now a properly split between the
actually-CONFIG_GDBSX stuff and the actually-CONFIG_DEBUG_CRASH stuff.

3) What is currently patch 1 wants to be next, taking with it the header
file rename from patch 2.

4) Finally, the remanent of patch 2.  The CONFIG_CRASH_DEBUG
implementation is now just the gdbstub call in _fatal(), so I don't
think a new debugger.c file is necessary.


Hopefully this all makes sense.

~Andrew

[qemu-mainline bisection] complete test-amd64-i386-xl-qemuu-ovmf-amd64

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-xl-qemuu-ovmf-amd64
testid debian-hvm-install

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://git.qemu.org/qemu.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  qemuu git://git.qemu.org/qemu.git
  Bug introduced:  d8fb7d0969d5c32b3d1b9e20b63ec6c0abe80be4
  Bug not present: c445909e1f3d5722ed26f067bbffed71cbefd711
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/163724/


  commit d8fb7d0969d5c32b3d1b9e20b63ec6c0abe80be4
  Author: Paolo Bonzini 
  Date:   Thu May 13 11:28:34 2021 -0400
  
  vl: switch -M parsing to keyval
  
  Switch from QemuOpts to keyval.  This enables the introduction
  of non-scalar machine properties, and JSON syntax in the future.
  
  For JSON syntax to be supported right now, we would have to
  consider what would happen if string-based dictionaries (produced by
  -M key=val) were to be merged with strongly-typed dictionaries
  (produced by -M {'key': 123}).
  
  The simplest way out is to never enter the situation, and only allow one
  -M option when JSON syntax is in use.  However, we want options such as
  -smp to become syntactic sugar for -M, and this is a problem; as soon
  as -smp becomes a shortcut for -M, QEMU would forbid using -M '{}'
  together with -smp.  Therefore, allowing JSON syntax right now for -M
  would be a forward-compatibility nightmare and it would be impossible
  anyway to introduce -M incrementally in tools.
  
  Instead, support for JSON syntax is delayed until after the main
  options are converted to QOM compound properties.  These include -boot,
  -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg.  Once JSON
  syntax is introduced, these options will _also_ be forbidden together
  with -M '{...}'.
  
  Signed-off-by: Paolo Bonzini 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu-mainline/test-amd64-i386-xl-qemuu-ovmf-amd64.debian-hvm-install.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/qemu-mainline/test-amd64-i386-xl-qemuu-ovmf-amd64.debian-hvm-install
 --summary-out=tmp/163724.bisection-summary --basis-template=163321 
--blessings=real,real-bisect,real-retry qemu-mainline 
test-amd64-i386-xl-qemuu-ovmf-amd64 debian-hvm-install
Searching for failure / basis pass:
 163694 fail [host=chardonnay0] / 163321 [host=huxelrebe1] 163311 [host=fiano0] 
163303 [host=huxelrebe0] 163299 [host=albana0] 163292 ok.
Failure / basis pass flights: 163694 / 163292
(tree with no url: minios)
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://git.qemu.org/qemu.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git
Latest c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c410ad4da4b7785170d3d42a3ba190c2caac6feb 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
1f966c7c11bbe77f3de5f50911de7c3a74594bfe 
54082c81d96028ba8c76fbe6784085cf1df76b20 
0f435e2b58543f5baae96e17a10ae20d3dbc28fa
Basis pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
c410ad4da4b7785170d3d42a3ba190c2caac6feb 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
9c2647f75004c4f7d64c9c0ec55f8c6f0739a8b1 
54082c81d96028ba8c76fbe6784085cf1df76b20 
74d044d51b19bb697eac5c3deafa140f6afafec8
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/linux-pvops.git#c3038e718a19fc596f7b1baba0f83d5146dc7784-c3038e718a19fc596f7b1baba0f83d5146dc7784
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/osstest/ovmf.git#c410ad4da4b7785170d3d42a3ba190c2caac6feb-c410ad4da4b7785170d3d42a3ba190c2caac6feb
 git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c74\
 37ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
git://git.qemu.org/qemu.git#9c2647f75004c4f7d64c9c0ec55f8c6f0739a8b1-1f966c7c11bbe77f3de5f50911de7c3a74594bfe
 
git://xenbits.xen.org/osstest/seabios.git#54082c81d96028ba8c76fbe6784085cf1df76b20-54082c81d96028ba8c76fbe6784085cf1df76b20
 
git://xenbits.xen.org/xen.git#74d044d51b19bb697eac5c3deafa140f6afafec8-0f435e2b58543f5baae96e17a10ae20d3dbc28fa
>From 

Re: [PATCH v2 07/10] xsm: drop generic event channel labeling

[Daniel P. Smith]

On 7/12/21 7:52 PM, Andrew Cooper wrote:
> On 12/07/2021 21:32, Daniel P. Smith wrote:
>> The generic event channel labeling has not been used by any XSM module since
>> its introduction. This commit removes the capability leaving FLASK labeling
>> field always present. In the future if a new XSM module needs to have its own
>> channel label, this or a new form can be introduced.
> 
> You're missing a SoB line.

Apologies, i was originally going to squash this but then decided it
probably served to be kept as a standalone commit. Will get the SoB added.

> Also, this too would benefit from being reordered higher than patch 6,
> to reduce the churn there.

Ack

v/r,
dps

Re: [PATCH v2 04/10] xsm: convert xsm_ops hook calls to alternative call

[Daniel P. Smith]

On 7/12/21 7:44 PM, Andrew Cooper wrote:
> On 12/07/2021 21:32, Daniel P. Smith wrote:
>> To reduce retpolines convert all the pointer function calls of the
>> xsm_ops hooks over to the alternative_call infrastructure.
>>
>> Signed-off-by: Daniel P. Smith 
>> ---
>>  xen/include/xsm/xsm.h | 195 +-
>>  1 file changed, 99 insertions(+), 96 deletions(-)
>>
>> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
>> index a8805f514b..a39b5dc42f 100644
>> --- a/xen/include/xsm/xsm.h
>> +++ b/xen/include/xsm/xsm.h
>> @@ -15,6 +15,9 @@
>>  #ifndef __XSM_H__
>>  #define __XSM_H__
>>  
>> +#ifdef CONFIG_XSM
>> +#include 
>> +#endif
> 
> This guard needs dropping to fix the build on ARM.

Ack

> Otherwise, Acked-by: Andrew Cooper 
> 

v/r,
dps

Re: [PATCH v2 03/10] xsm: remove the ability to disable flask

[Daniel P. Smith]

On 7/14/21 11:58 AM, Jan Beulich wrote:
> On 12.07.2021 22:32, Daniel P. Smith wrote:
>> The flask XSM module provided the ability to switch from flask back to
>> the dummy XSM module during runtime. With this removal the only way to
>> switch between XSM modules is at boot time.
>>
>> Signed-off-by: Daniel P. Smith 
> 
> Can you please add a statement as to why you/we want this, perhaps
> helping clarify why apparently you think no-one is making use of
> this?

Yes, I can expand more on the why.

v/r
dps

Re: [PATCH v2 03/10] xsm: remove the ability to disable flask

[Daniel P. Smith]

On 7/12/21 7:22 PM, Andrew Cooper wrote:
> On 12/07/2021 21:32, Daniel P. Smith wrote:
>> The flask XSM module provided the ability to switch from flask back to
>> the dummy XSM module during runtime. With this removal the only way to
>> switch between XSM modules is at boot time.
>>
>> Signed-off-by: Daniel P. Smith 
> 
> This patch wants reordering ahead of "xsm: refactor xsm_ops handling"
> which will reduce the churn in that patch.

Ack

> In addition, you want:
> 
> diff --git a/xen/include/public/xsm/flask_op.h
> b/xen/include/public/xsm/flask_op.h
> index 16af7bc22f75..b41dd6dac894 100644
> --- a/xen/include/public/xsm/flask_op.h
> +++ b/xen/include/public/xsm/flask_op.h
> @@ -188,7 +188,7 @@ struct xen_flask_op {
>  #define FLASK_SETBOOL   12
>  #define FLASK_COMMITBOOLS   13
>  #define FLASK_MLS   14
> -#define FLASK_DISABLE   15
> +#define FLASK_DISABLE   15 /* No longer implemented */
>  #define FLASK_GETAVC_THRESHOLD  16
>  #define FLASK_SETAVC_THRESHOLD  17
>  #define FLASK_AVC_HASHSTATS 18
> 
> to match the removal of FLASK_USER in c/s 559f439bfa3bf

Ack

Re: preparations for 4.15.1 and 4.13.4

[Andrew Cooper]

On 15/07/2021 08:58, Jan Beulich wrote:
> Beyond this I'd like the following to be considered:
>
> 6409210a5f51 libxencall: osdep_hypercall() should return long
> bef64f2c0019 libxencall: introduce variant of xencall2() returning long
> 01a2d001dea2 libxencall: Bump SONAME following new functionality
> 6f02d1ea4a10 libxc: use multicall for memory-op on Linux (and Solaris)
>
> If those are to be taken (which means in particular if the question of
> the .so versioning can be properly sorted),
>
> 198a2bc6f149 x86/HVM: wire up multicalls

We can backport changes in SONAME safely so long as:

1) We declare VERS_1.2 to be fixed and released.  This means that we
bump to 1.3 for the next change, even if it is ahead of Xen 4.16 being
release, and

2) *All* ABI changes up to VERS_1.2 are backported.


The ABI called VERS_1.2 must be identical on all older branches to avoid
binary problems when rebuilding a package against old-xen+updates, and
then updating to a newer Xen.

~Andrew

Re: [PATCH v2 02/10] xsm: refactor xsm_ops handling

[Daniel P. Smith]

On 7/12/21 7:39 PM, Andrew Cooper wrote:
> On 12/07/2021 21:32, Daniel P. Smith wrote:
>> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
>> index ad3cddbf7d..a8805f514b 100644
>> --- a/xen/include/xsm/xsm.h
>> +++ b/xen/include/xsm/xsm.h
>> @@ -747,16 +747,14 @@ extern int xsm_dt_policy_init(void **policy_buffer, 
>> size_t *policy_size);
>>  extern bool has_xsm_magic(paddr_t);
>>  #endif
>>  
>> -extern int register_xsm(struct xsm_operations *ops);
>> -
>> -extern struct xsm_operations dummy_xsm_ops;
>>  extern void xsm_fixup_ops(struct xsm_operations *ops);
>>  
>>  #ifdef CONFIG_XSM_FLASK
>> -extern void flask_init(const void *policy_buffer, size_t policy_size);
>> +extern struct xsm_operations *flask_init(const void *policy_buffer, size_t 
>> policy_size);
>>  #else
>> -static inline void flask_init(const void *policy_buffer, size_t policy_size)
>> +static inline struct xsm_operations *flask_init(const void *policy_buffer, 
>> size_t policy_size)
>>  {
>> +return NULL;
>>  }
>>  #endif
> 
> As you touch almost every current user of xsm_operations and introduce
> quite a few more, can I recommend taking the opportunity to shorten the
> name to struct xsm_ops.

Looks like Jan also agreed, so I will add this to the mix.

>> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
>> index 01e52138a1..32e079d676 100644
>> --- a/xen/xsm/flask/flask_op.c
>> +++ b/xen/xsm/flask/flask_op.c
>> @@ -226,6 +226,7 @@ static int flask_security_sid(struct 
>> xen_flask_sid_context *arg)
>>  static int flask_disable(void)
>>  {
>>  static int flask_disabled = 0;
>> +struct xsm_operations default_ops;
>>  
>>  if ( ss_initialized )
>>  {
>> @@ -244,7 +245,8 @@ static int flask_disable(void)
>>  flask_disabled = 1;
>>  
>>  /* Reset xsm_ops to the original module. */
>> -xsm_ops = _xsm_ops;
>> +xsm_fixup_ops(_ops);
>> +xsm_ops = default_ops;
>>  
>>  return 0;
>>  }
> 
> These two hunks will disappear when patch 3 is reordered with respect to
> this one.
> 
> ... which is good because you've just pointed xsm_ops at a
> soon-to-be-out-of-scope local variable on the stack.
> 

As Jan has pointed out it is not a ref issue, but it was very bad of me
to leave the stack var uninitialized. Regardless as you pointed out,
this will be irrelevant with moving patch 3 ahead of this.

>> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
>> index f1a1217c98..a3a88aa8ed 100644
>> --- a/xen/xsm/flask/hooks.c
>> +++ b/xen/xsm/flask/hooks.c
>> @@ -1883,7 +1883,8 @@ static struct xsm_operations flask_ops = {
>>  #endif
>>  };
> 
> flask and silo ops should become:
> 
> static const struct xsm_ops __initconst {flask,silo}_ops = {
> 
> as they're neither modified, nor needed after init, following this change.
> 

After this and seeing Jan's comments, I am going to walk through this
again and see if there is more adjustments/cleanups I can do/

>>  
>> -void __init flask_init(const void *policy_buffer, size_t policy_size)
>> +struct xsm_operations __init *flask_init(const void *policy_buffer,
>> + size_t policy_size)
> 
> struct xsm_ops *__init flask_init(...)
> 
> Otherwise you've got the __init in the middle of the return type, and
> some compilers are more picky than others.

Ack

>> @@ -1923,6 +1921,9 @@ void __init flask_init(const void *policy_buffer, 
>> size_t policy_size)
>>  printk(XENLOG_INFO "Flask:  Starting in enforcing mode.\n");
>>  else
>>  printk(XENLOG_INFO "Flask:  Starting in permissive mode.\n");
>> +
>> +return _ops;
>> +
> 
> Stray newline.

Ack

>>  }
>>  
>>  /*
>> diff --git a/xen/xsm/silo.c b/xen/xsm/silo.c
>> index fc2ca5cd2d..808350f122 100644
>> --- a/xen/xsm/silo.c
>> +++ b/xen/xsm/silo.c
>> @@ -112,12 +112,11 @@ static struct xsm_operations silo_xsm_ops = {
>>  #endif
>>  };
>>  
>> -void __init silo_init(void)
>> +struct xsm_operations __init *silo_init(void)
> 
> Same here as with flask.

Ack

>> diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
>> index 5eab21e1b1..7265f742e9 100644
>> --- a/xen/xsm/xsm_core.c
>> +++ b/xen/xsm/xsm_core.c
>> @@ -68,17 +76,10 @@ static int __init parse_xsm_param(const char *s)
>>  }
>>  custom_param("xsm", parse_xsm_param);
>>  
>> -static inline int verify(struct xsm_operations *ops)
>> -{
>> -/* verify the security_operations structure exists */
>> -if ( !ops )
>> -return -EINVAL;
>> -xsm_fixup_ops(ops);
>> -return 0;
>> -}
>> -
>>  static int __init xsm_core_init(const void *policy_buffer, size_t 
>> policy_size)
>>  {
>> +struct xsm_operations *mod_ops;
>> +
> 
> Hard tabs, and later in this function.  Also, how about just 'ops' for
> the local variable name?

Ack

>> @@ -113,6 +124,17 @@ static int __init xsm_core_init(const void 
>> *policy_buffer, size_t policy_size)
>>  break;
>>  }
>>  
>> +/*
>> + * This handles three cases,
>> + *   - dummy policy module was selected
>> + *  

Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses

[Christoph Hellwig]

On Thu, Jul 15, 2021 at 12:58:53PM -0400, Boris Ostrovsky wrote:
> 
> On 7/15/21 3:39 AM, Roman Skakun wrote:
> >> This looks like it wasn't picked up? Should it go in rc1?
> > Hi, Konrad!
> >
> > This looks like an unambiguous bug, and should be in rc1.
> 
> 
> Looks like you didn't copy Christoph which could be part of the problem. 
> Adding him.

Can someone just send a clean patch that I can review and hopefully
apply?

Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses

[Boris Ostrovsky]

On 7/15/21 3:39 AM, Roman Skakun wrote:
>> This looks like it wasn't picked up? Should it go in rc1?
> Hi, Konrad!
>
> This looks like an unambiguous bug, and should be in rc1.


Looks like you didn't copy Christoph which could be part of the problem. Adding 
him.


-boris



>
> Cheers!
>
> ср, 14 июл. 2021 г. в 03:15, Konrad Rzeszutek Wilk :
>> On Tue, Jun 22, 2021 at 04:34:14PM +0300, Roman Skakun wrote:
>>> This commit is dedicated to fix incorrect conversion from
>>> cpu_addr to page address in cases when we get virtual
>>> address which allocated in the vmalloc range.
>>> As the result, virt_to_page() cannot convert this address
>>> properly and return incorrect page address.
>>>
>>> Need to detect such cases and obtains the page address using
>>> vmalloc_to_page() instead.
>>>
>>> Signed-off-by: Roman Skakun 
>>> Reviewed-by: Andrii Anisov 
>>> ---
>>> Hey!
>>> Thanks for suggestions, Christoph!
>>> I updated the patch according to your advice.
>>> But, I'm so surprised because nobody catches this problem
>>> in the common code before. It looks a bit strange as for me.
>> This looks like it wasn't picked up? Should it go in rc1?
>>>
>>>  kernel/dma/ops_helpers.c | 12 ++--
>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/dma/ops_helpers.c b/kernel/dma/ops_helpers.c
>>> index 910ae69cae77..782728d8a393 100644
>>> --- a/kernel/dma/ops_helpers.c
>>> +++ b/kernel/dma/ops_helpers.c
>>> @@ -5,6 +5,14 @@
>>>   */
>>>  #include 
>>>
>>> +static struct page *cpu_addr_to_page(void *cpu_addr)
>>> +{
>>> + if (is_vmalloc_addr(cpu_addr))
>>> + return vmalloc_to_page(cpu_addr);
>>> + else
>>> + return virt_to_page(cpu_addr);
>>> +}
>>> +
>>>  /*
>>>   * Create scatter-list for the already allocated DMA buffer.
>>>   */
>>> @@ -12,7 +20,7 @@ int dma_common_get_sgtable(struct device *dev, struct 
>>> sg_table *sgt,
>>>void *cpu_addr, dma_addr_t dma_addr, size_t size,
>>>unsigned long attrs)
>>>  {
>>> - struct page *page = virt_to_page(cpu_addr);
>>> + struct page *page = cpu_addr_to_page(cpu_addr);
>>>   int ret;
>>>
>>>   ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
>>> @@ -43,7 +51,7 @@ int dma_common_mmap(struct device *dev, struct 
>>> vm_area_struct *vma,
>>>   return -ENXIO;
>>>
>>>   return remap_pfn_range(vma, vma->vm_start,
>>> - page_to_pfn(virt_to_page(cpu_addr)) + vma->vm_pgoff,
>>> + page_to_pfn(cpu_addr_to_page(cpu_addr)) + 
>>> vma->vm_pgoff,
>>>   user_count << PAGE_SHIFT, vma->vm_page_prot);
>>>  #else
>>>   return -ENXIO;
>>> --
>>> 2.25.1
>>>
>
>

[PATCH v1 05/16] alpha: return error code from alpha_pci_map_sg()

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

pci_map_single_1() can fail for different reasons, but since the only
supported type of error return is DMA_MAPPING_ERROR, we coalesce those
errors into EINVAL.

ENOMEM is returned when no page tables can be allocated.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Richard Henderson 
Cc: Ivan Kokshaysky 
Cc: Matt Turner 
---
 arch/alpha/kernel/pci_iommu.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index 35d7b3096d6e..72fc2465d13c 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -649,7 +649,9 @@ static int alpha_pci_map_sg(struct device *dev, struct 
scatterlist *sg,
sg->dma_address
  = pci_map_single_1(pdev, SG_ENT_VIRT_ADDRESS(sg),
 sg->length, dac_allowed);
-   return sg->dma_address != DMA_MAPPING_ERROR;
+   if (sg->dma_address == DMA_MAPPING_ERROR)
+   return -EINVAL;
+   return 1;
}
 
start = sg;
@@ -685,8 +687,10 @@ static int alpha_pci_map_sg(struct device *dev, struct 
scatterlist *sg,
if (out < end)
out->dma_length = 0;
 
-   if (out - start == 0)
+   if (out - start == 0) {
printk(KERN_WARNING "pci_map_sg failed: no entries?\n");
+   return -ENOMEM;
+   }
DBGA("pci_map_sg: %ld entries\n", out - start);
 
return out - start;
@@ -699,7 +703,7 @@ static int alpha_pci_map_sg(struct device *dev, struct 
scatterlist *sg,
   entries.  Unmap them now.  */
if (out > start)
pci_unmap_sg(pdev, start, out - start, dir);
-   return 0;
+   return -ENOMEM;
 }
 
 /* Unmap a set of streaming mode DMA translations.  Again, cpu read
-- 
2.20.1

Re: [PATCH v1 00/16] .map_sg() error cleanup

[Logan Gunthorpe]

On 2021-07-15 10:53 a.m., Russell King (Oracle) wrote:
> On Thu, Jul 15, 2021 at 10:45:28AM -0600, Logan Gunthorpe wrote:
>> Hi,
>>
>> This series is spun out and expanded from my work to add P2PDMA support
>> to DMA map operations[1].
>>
>> The P2PDMA work requires distinguishing different error conditions in
>> a map_sg operation. dma_map_sgtable() already allows for returning an
>> error code (where as dma_map_sg() is only allowed to return zero)
>> however, it currently only returns -EINVAL when a .map_sg() call returns
>> zero.
>>
>> This series cleans up all .map_sg() implementations to return appropriate
>> error codes. After the cleanup, dma_map_sg() will still return zero,
>> however dma_map_sgtable() will pass the error code from the .map_sg()
>> call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the
>> obscure implementations.
>>
>> The patch set is based off of v5.14-rc1 and a git repo can be found
>> here:
> 
> Have all the callers for dma_map_sg() been updated to check for error
> codes? If not, isn't that a pre-requisit to this patch set?

No. Perhaps I wasn't clear enough: This series is changing only
impelemntations of .map_sg(). It does *not* change the return code of
dma_map_sg(). dma_map_sg() will continue to return zero on error for the
foreseeable future. The dma_map_sgtable() call already allows returning
error codes and it will pass the new error code through. This is what
will be used in the P2PDMA work.

Logan

Re: [PATCH v1 00/16] .map_sg() error cleanup

[Russell King (Oracle)]

On Thu, Jul 15, 2021 at 10:45:28AM -0600, Logan Gunthorpe wrote:
> Hi,
> 
> This series is spun out and expanded from my work to add P2PDMA support
> to DMA map operations[1].
> 
> The P2PDMA work requires distinguishing different error conditions in
> a map_sg operation. dma_map_sgtable() already allows for returning an
> error code (where as dma_map_sg() is only allowed to return zero)
> however, it currently only returns -EINVAL when a .map_sg() call returns
> zero.
> 
> This series cleans up all .map_sg() implementations to return appropriate
> error codes. After the cleanup, dma_map_sg() will still return zero,
> however dma_map_sgtable() will pass the error code from the .map_sg()
> call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the
> obscure implementations.
> 
> The patch set is based off of v5.14-rc1 and a git repo can be found
> here:

Have all the callers for dma_map_sg() been updated to check for error
codes? If not, isn't that a pre-requisit to this patch set?

>From what I see in Linus' current tree, we still have cases today
where the return value of dma_map_sg() is compared with zero to
detect failure, so I think that needs fixing before we start changing
the dma_map_sg() implementation to return negative numbers.

I also notice that there are various places that don't check the
return value - and returning a negative number instead of zero may
well cause random other bits to be set in fields.

So, I think there's a fair amount of work to do in all the drivers
before this change can be considered.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

[PATCH v1 06/16] ARM/dma-mapping: return error code from .map_sg() ops

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure,
so propagate any errors that may happen all the way up.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Russell King 
Cc: Thomas Bogendoerfer 
---
 arch/arm/mm/dma-mapping.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c4b8df2ad328..8c286e690756 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -980,7 +980,7 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist 
*sg, int nents,
 {
const struct dma_map_ops *ops = get_dma_ops(dev);
struct scatterlist *s;
-   int i, j;
+   int i, j, ret;
 
for_each_sg(sg, s, nents, i) {
 #ifdef CONFIG_NEED_SG_DMA_LENGTH
@@ -988,7 +988,8 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist 
*sg, int nents,
 #endif
s->dma_address = ops->map_page(dev, sg_page(s), s->offset,
s->length, dir, attrs);
-   if (dma_mapping_error(dev, s->dma_address))
+   ret = dma_mapping_error(dev, s->dma_address);
+   if (ret)
goto bad_mapping;
}
return nents;
@@ -996,7 +997,7 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist 
*sg, int nents,
  bad_mapping:
for_each_sg(sg, s, i, j)
ops->unmap_page(dev, sg_dma_address(s), sg_dma_len(s), dir, 
attrs);
-   return 0;
+   return ret;
 }
 
 /**
@@ -1622,7 +1623,7 @@ static int __iommu_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
 bool is_coherent)
 {
struct scatterlist *s = sg, *dma = sg, *start = sg;
-   int i, count = 0;
+   int i, count = 0, ret;
unsigned int offset = s->offset;
unsigned int size = s->offset + s->length;
unsigned int max = dma_get_max_seg_size(dev);
@@ -1634,8 +1635,10 @@ static int __iommu_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
s->dma_length = 0;
 
if (s->offset || (size & ~PAGE_MASK) || size + s->length > max) 
{
-   if (__map_sg_chunk(dev, start, size, >dma_address,
-   dir, attrs, is_coherent) < 0)
+   ret = __map_sg_chunk(dev, start, size,
+>dma_address, dir, attrs,
+is_coherent);
+   if (ret < 0)
goto bad_mapping;
 
dma->dma_address += offset;
@@ -1648,8 +1651,9 @@ static int __iommu_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
}
size += s->length;
}
-   if (__map_sg_chunk(dev, start, size, >dma_address, dir, attrs,
-   is_coherent) < 0)
+   ret = __map_sg_chunk(dev, start, size, >dma_address, dir, attrs,
+is_coherent);
+   if (ret < 0)
goto bad_mapping;
 
dma->dma_address += offset;
@@ -1660,7 +1664,7 @@ static int __iommu_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
 bad_mapping:
for_each_sg(sg, s, count, i)
__iommu_remove_mapping(dev, sg_dma_address(s), sg_dma_len(s));
-   return 0;
+   return ret;
 }
 
 /**
-- 
2.20.1

[PATCH v1 07/16] ia64/sba_iommu: return error code from sba_map_sg_attrs()

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

Propagate the return of dma_mapping_error() up, if it is an errno.

sba_coalesce_chunks() may only presently fail if sba_alloc_range()
fails, which in turn only fails if the iommu is out of mapping
resources, hence a -ENOMEM is used in that case.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Michael Ellerman 
Cc: Niklas Schnelle 
Cc: Thomas Bogendoerfer 
---
 arch/ia64/hp/common/sba_iommu.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 9148ddbf02e5..09dbe07a18c1 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1431,7 +1431,7 @@ static int sba_map_sg_attrs(struct device *dev, struct 
scatterlist *sglist,
unsigned long attrs)
 {
struct ioc *ioc;
-   int coalesced, filled = 0;
+   int coalesced, filled = 0, ret;
 #ifdef ASSERT_PDIR_SANITY
unsigned long flags;
 #endif
@@ -1458,8 +1458,9 @@ static int sba_map_sg_attrs(struct device *dev, struct 
scatterlist *sglist,
sglist->dma_length = sglist->length;
sglist->dma_address = sba_map_page(dev, sg_page(sglist),
sglist->offset, sglist->length, dir, attrs);
-   if (dma_mapping_error(dev, sglist->dma_address))
-   return 0;
+   ret = dma_mapping_error(dev, sglist->dma_address);
+   if (ret)
+   return ret;
return 1;
}
 
@@ -1486,7 +1487,7 @@ static int sba_map_sg_attrs(struct device *dev, struct 
scatterlist *sglist,
coalesced = sba_coalesce_chunks(ioc, dev, sglist, nents);
if (coalesced < 0) {
sba_unmap_sg_attrs(dev, sglist, nents, dir, attrs);
-   return 0;
+   return -ENOMEM;
}
 
/*
-- 
2.20.1

[PATCH v1 09/16] powerpc/iommu: return error code from .map_sg() ops

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

Propagate the error up if vio_dma_iommu_map_sg() fails.

ppc_iommu_map_sg() may fail either because of iommu_range_alloc() or
because of tbl->it_ops->set(). The former only supports returning an
error with DMA_MAPPING_ERROR and an examination of the latter indicates
that it may return arch-specific errors (for example,
tce_buildmulti_pSeriesLP()). Hence, coalesce all of those errors into
-EINVAL;

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Geoff Levand 
---
 arch/powerpc/kernel/iommu.c | 4 ++--
 arch/powerpc/platforms/ps3/system-bus.c | 2 +-
 arch/powerpc/platforms/pseries/vio.c| 5 +++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 2af89a5e379f..bd0ed618bfa5 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -473,7 +473,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table 
*tbl,
BUG_ON(direction == DMA_NONE);
 
if ((nelems == 0) || !tbl)
-   return 0;
+   return -EINVAL;
 
outs = s = segstart = [0];
outcount = 1;
@@ -599,7 +599,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table 
*tbl,
if (s == outs)
break;
}
-   return 0;
+   return -EINVAL;
 }
 
 
diff --git a/arch/powerpc/platforms/ps3/system-bus.c 
b/arch/powerpc/platforms/ps3/system-bus.c
index 1a5665875165..c54eb46f0cfb 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -663,7 +663,7 @@ static int ps3_ioc0_map_sg(struct device *_dev, struct 
scatterlist *sg,
   unsigned long attrs)
 {
BUG();
-   return 0;
+   return -EINVAL;
 }
 
 static void ps3_sb_unmap_sg(struct device *_dev, struct scatterlist *sg,
diff --git a/arch/powerpc/platforms/pseries/vio.c 
b/arch/powerpc/platforms/pseries/vio.c
index e00f3725ec96..e31e59c54f30 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -560,7 +560,8 @@ static int vio_dma_iommu_map_sg(struct device *dev, struct 
scatterlist *sglist,
for_each_sg(sglist, sgl, nelems, count)
alloc_size += roundup(sgl->length, IOMMU_PAGE_SIZE(tbl));
 
-   if (vio_cmo_alloc(viodev, alloc_size))
+   ret = vio_cmo_alloc(viodev, alloc_size);
+   if (ret)
goto out_fail;
ret = ppc_iommu_map_sg(dev, tbl, sglist, nelems, dma_get_mask(dev),
direction, attrs);
@@ -577,7 +578,7 @@ static int vio_dma_iommu_map_sg(struct device *dev, struct 
scatterlist *sglist,
vio_cmo_dealloc(viodev, alloc_size);
 out_fail:
atomic_inc(>cmo.allocs_failed);
-   return 0;
+   return ret;
 }
 
 static void vio_dma_iommu_unmap_sg(struct device *dev,
-- 
2.20.1

[PATCH v1 08/16] MIPS/jazzdma: return error code from jazz_dma_map_sg()

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

vdma_alloc() may fail for different reasons, but since it only supports
indicating an error via a return of DMA_MAPPING_ERROR, we coalesce the
different reasons into -EINVAL.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Thomas Bogendoerfer 
---
 arch/mips/jazz/jazzdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/jazz/jazzdma.c b/arch/mips/jazz/jazzdma.c
index 461457b28982..3b99743435db 100644
--- a/arch/mips/jazz/jazzdma.c
+++ b/arch/mips/jazz/jazzdma.c
@@ -552,7 +552,7 @@ static int jazz_dma_map_sg(struct device *dev, struct 
scatterlist *sglist,
dir);
sg->dma_address = vdma_alloc(sg_phys(sg), sg->length);
if (sg->dma_address == DMA_MAPPING_ERROR)
-   return 0;
+   return -EINVAL;
sg_dma_len(sg) = sg->length;
}

--
2.20.1

[PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg()

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

So make __dma_map_cont() return a valid errno (which is then propagated
to gart_map_sg() via dma_map_cont()) and return it in case of failure.

Also, return -EINVAL in case of invalid nents.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: "H. Peter Anvin" 
Cc: Niklas Schnelle 
Cc: Thomas Bogendoerfer 
Cc: Michael Ellerman 
---
 arch/x86/kernel/amd_gart_64.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index 9ac696487b13..46aea9a4f26b 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -331,7 +331,7 @@ static int __dma_map_cont(struct device *dev, struct 
scatterlist *start,
int i;
 
if (iommu_start == -1)
-   return -1;
+   return -ENOMEM;
 
for_each_sg(start, s, nelems, i) {
unsigned long pages, addr;
@@ -380,13 +380,13 @@ static int gart_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
   enum dma_data_direction dir, unsigned long attrs)
 {
struct scatterlist *s, *ps, *start_sg, *sgmap;
-   int need = 0, nextneed, i, out, start;
+   int need = 0, nextneed, i, out, start, ret;
unsigned long pages = 0;
unsigned int seg_size;
unsigned int max_seg_size;
 
if (nents == 0)
-   return 0;
+   return -EINVAL;
 
out = 0;
start   = 0;
@@ -414,8 +414,9 @@ static int gart_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
if (!iommu_merge || !nextneed || !need || s->offset ||
(s->length + seg_size > max_seg_size) ||
(ps->offset + ps->length) % PAGE_SIZE) {
-   if (dma_map_cont(dev, start_sg, i - start,
-sgmap, pages, need) < 0)
+   ret = dma_map_cont(dev, start_sg, i - start,
+  sgmap, pages, need);
+   if (ret < 0)
goto error;
out++;
 
@@ -432,7 +433,8 @@ static int gart_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
pages += iommu_num_pages(s->offset, s->length, PAGE_SIZE);
ps = s;
}
-   if (dma_map_cont(dev, start_sg, i - start, sgmap, pages, need) < 0)
+   ret = dma_map_cont(dev, start_sg, i - start, sgmap, pages, need);
+   if (ret < 0)
goto error;
out++;
flush_gart();
@@ -458,7 +460,7 @@ static int gart_map_sg(struct device *dev, struct 
scatterlist *sg, int nents,
iommu_full(dev, pages << PAGE_SHIFT, dir);
for_each_sg(sg, s, nents, i)
s->dma_address = DMA_MAPPING_ERROR;
-   return 0;
+   return ret;
 }
 
 /* allocate and map a coherent mapping */
-- 
2.20.1

[PATCH v1 12/16] parisc: return error code from .map_sg() ops

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
---
 drivers/parisc/ccio-dma.c  | 2 +-
 drivers/parisc/sba_iommu.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index b5f9ee81a46c..a3a5cfda3d93 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -918,7 +918,7 @@ ccio_map_sg(struct device *dev, struct scatterlist *sglist, 
int nents,
BUG_ON(!dev);
ioc = GET_IOC(dev);
if (!ioc)
-   return 0;
+   return -ENODEV;

DBG_RUN_SG("%s() START %d entries\n", __func__, nents);
 
diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index dce4cdf786cd..9a6671a230ee 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -947,7 +947,7 @@ sba_map_sg(struct device *dev, struct scatterlist *sglist, 
int nents,
 
ioc = GET_IOC(dev);
if (!ioc)
-   return 0;
+   return -ENODEV;
 
/* Fast path single entry scatterlists. */
if (nents == 1) {
-- 
2.20.1

[PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg()

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

So propagate the error from __s390_dma_map_sg() up.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Niklas Schnelle 
Cc: Gerald Schaefer 
Cc: Heiko Carstens 
Cc: Vasily Gorbik 
Cc: Christian Borntraeger 
---
 arch/s390/pci/pci_dma.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index ebc9a49523aa..c78b02012764 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -487,7 +487,7 @@ static int s390_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
unsigned int max = dma_get_max_seg_size(dev);
unsigned int size = s->offset + s->length;
unsigned int offset = s->offset;
-   int count = 0, i;
+   int count = 0, i, ret;
 
for (i = 1; i < nr_elements; i++) {
s = sg_next(s);
@@ -497,8 +497,9 @@ static int s390_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
 
if (s->offset || (size & ~PAGE_MASK) ||
size + s->length > max) {
-   if (__s390_dma_map_sg(dev, start, size,
- >dma_address, dir))
+   ret = __s390_dma_map_sg(dev, start, size,
+   >dma_address, dir);
+   if (ret)
goto unmap;
 
dma->dma_address += offset;
@@ -511,7 +512,8 @@ static int s390_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
}
size += s->length;
}
-   if (__s390_dma_map_sg(dev, start, size, >dma_address, dir))
+   ret = __s390_dma_map_sg(dev, start, size, >dma_address, dir);
+   if (ret)
goto unmap;
 
dma->dma_address += offset;
@@ -523,7 +525,7 @@ static int s390_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
s390_dma_unmap_pages(dev, sg_dma_address(s), sg_dma_len(s),
 dir, attrs);
 
-   return 0;
+   return ret;
 }
 
 static void s390_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-- 
2.20.1

[PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error

[Logan Gunthorpe]

Now that all the .map_sg operations have been converted to returning
proper error codes, drop the code to handle a zero return value,
add a warning if a zero is returned and update the comment for the
map_sg operation.

Signed-off-by: Logan Gunthorpe 
---
 include/linux/dma-map-ops.h | 8 +++-
 kernel/dma/mapping.c| 6 +++---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index eaa969be8284..f299bc1e317b 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -42,11 +42,9 @@ struct dma_map_ops {
unsigned long attrs);
/*
 * map_sg should return a negative error code on error.
-* dma_map_sgtable() will return the error code returned and convert
-* a zero return (for legacy implementations) into -EINVAL.
-*
-* dma_map_sg() will always return zero on any negative or zero
-* return to satisfy its own calling convention.
+* dma_map_sgtable() will return the error code returned by the
+* operation and dma_map_sg() will always convert any error to zero
+* to satisfy its own calling convention.
 */
int (*map_sg)(struct device *dev, struct scatterlist *sg, int nents,
enum dma_data_direction dir, unsigned long attrs);
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 30f89d244566..978a6a16aaf7 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -194,6 +194,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct 
scatterlist *sg,
else
ents = ops->map_sg(dev, sg, nents, dir, attrs);
 
+   WARN_ON_ONCE(ents == 0);
+
if (ents > 0)
debug_dma_map_sg(dev, sg, nents, ents, dir);
 
@@ -251,9 +253,7 @@ int dma_map_sgtable(struct device *dev, struct sg_table 
*sgt,
int nents;
 
nents = __dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
-   if (nents == 0)
-   return -EINVAL;
-   else if (nents < 0)
+   if (nents < 0)
return nents;
 
sgt->nents = nents;
-- 
2.20.1

[PATCH v1 11/16] sparc/iommu: return error codes from .map_sg() ops

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

Returning an errno from __sbus_iommu_map_sg() results in
sbus_iommu_map_sg_gflush() and sbus_iommu_map_sg_pflush() returning an
errno, as those functions are wrappers around __sbus_iommu_map_sg().

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: "David S. Miller" 
Cc: Niklas Schnelle 
Cc: Michael Ellerman 
---
 arch/sparc/kernel/iommu.c | 4 ++--
 arch/sparc/kernel/pci_sun4v.c | 4 ++--
 arch/sparc/mm/iommu.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index a034f571d869..0589acd34201 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -448,7 +448,7 @@ static int dma_4u_map_sg(struct device *dev, struct 
scatterlist *sglist,
iommu = dev->archdata.iommu;
strbuf = dev->archdata.stc;
if (nelems == 0 || !iommu)
-   return 0;
+   return -EINVAL;
 
spin_lock_irqsave(>lock, flags);
 
@@ -580,7 +580,7 @@ static int dma_4u_map_sg(struct device *dev, struct 
scatterlist *sglist,
}
spin_unlock_irqrestore(>lock, flags);
 
-   return 0;
+   return -EINVAL;
 }
 
 /* If contexts are being used, they are the same in all of the mappings
diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
index 9de57e88f7a1..d90e80fa5705 100644
--- a/arch/sparc/kernel/pci_sun4v.c
+++ b/arch/sparc/kernel/pci_sun4v.c
@@ -486,7 +486,7 @@ static int dma_4v_map_sg(struct device *dev, struct 
scatterlist *sglist,
 
iommu = dev->archdata.iommu;
if (nelems == 0 || !iommu)
-   return 0;
+   return -EINVAL;
atu = iommu->atu;
 
prot = HV_PCI_MAP_ATTR_READ;
@@ -619,7 +619,7 @@ static int dma_4v_map_sg(struct device *dev, struct 
scatterlist *sglist,
}
local_irq_restore(flags);
 
-   return 0;
+   return -EINVAL;
 }
 
 static void dma_4v_unmap_sg(struct device *dev, struct scatterlist *sglist,
diff --git a/arch/sparc/mm/iommu.c b/arch/sparc/mm/iommu.c
index 0c0342e5b10d..01ffcedd159c 100644
--- a/arch/sparc/mm/iommu.c
+++ b/arch/sparc/mm/iommu.c
@@ -256,7 +256,7 @@ static int __sbus_iommu_map_sg(struct device *dev, struct 
scatterlist *sgl,
sg->dma_address =__sbus_iommu_map_page(dev, sg_page(sg),
sg->offset, sg->length, per_page_flush);
if (sg->dma_address == DMA_MAPPING_ERROR)
-   return 0;
+   return -EINVAL;
sg->dma_length = sg->length;
}
 
-- 
2.20.1

[PATCH v1 15/16] dma-mapping: return error code from dma_dummy_map_sg()

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

The only errno to return is -ENODEV in the case when DMA is not
supported.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
---
 kernel/dma/dummy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/dummy.c b/kernel/dma/dummy.c
index eacd4c5b10bf..ae9abebed0c4 100644
--- a/kernel/dma/dummy.c
+++ b/kernel/dma/dummy.c
@@ -22,7 +22,7 @@ static int dma_dummy_map_sg(struct device *dev, struct 
scatterlist *sgl,
int nelems, enum dma_data_direction dir,
unsigned long attrs)
 {
-   return 0;
+   return -ENODEV;
 }
 
 static int dma_dummy_supported(struct device *hwdev, u64 mask)
-- 
2.20.1

[PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg()

[Logan Gunthorpe]

From: Martin Oliveira 

The .map_sg() op now expects an error code instead of zero on failure.

xen_swiotlb_map_sg() may only fail if xen_swiotlb_map_page() fails, but
xen_swiotlb_map_page() only supports returning errors as
DMA_MAPPING_ERROR. So coalesce all errors into EINVAL.

Signed-off-by: Martin Oliveira 
Signed-off-by: Logan Gunthorpe 
Cc: Konrad Rzeszutek Wilk 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Stefano Stabellini 
---
 drivers/xen/swiotlb-xen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 24d11861ac7d..b5707127c9d7 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -509,7 +509,7 @@ xen_swiotlb_map_sg(struct device *dev, struct scatterlist 
*sgl, int nelems,
 out_unmap:
xen_swiotlb_unmap_sg(dev, sgl, i, dir, attrs | DMA_ATTR_SKIP_CPU_SYNC);
sg_dma_len(sgl) = 0;
-   return 0;
+   return -EINVAL;
 }
 
 static void
-- 
2.20.1

[PATCH v1 00/16] .map_sg() error cleanup

[Logan Gunthorpe]

Hi,

This series is spun out and expanded from my work to add P2PDMA support
to DMA map operations[1].

The P2PDMA work requires distinguishing different error conditions in
a map_sg operation. dma_map_sgtable() already allows for returning an
error code (where as dma_map_sg() is only allowed to return zero)
however, it currently only returns -EINVAL when a .map_sg() call returns
zero.

This series cleans up all .map_sg() implementations to return appropriate
error codes. After the cleanup, dma_map_sg() will still return zero,
however dma_map_sgtable() will pass the error code from the .map_sg()
call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the
obscure implementations.

The patch set is based off of v5.14-rc1 and a git repo can be found
here:

  https://github.com/sbates130272/linux-p2pmem map_sg_err_cleanup_v1

Thanks,

Logan

[1] 
https://lore.kernel.org/linux-block/20210513223203.5542-1-log...@deltatee.com/

--

Logan Gunthorpe (5):
  dma-mapping: Allow map_sg() ops to return negative error codes
  dma-direct: Return appropriate error code from dma_direct_map_sg()
  iommu: Return full error code from iommu_map_sg[_atomic]()
  dma-iommu: Return error code from iommu_dma_map_sg()
  dma-mapping: Disallow .map_sg operations from returning zero on error

Martin Oliveira (11):
  alpha: return error code from alpha_pci_map_sg()
  ARM/dma-mapping: return error code from .map_sg() ops
  ia64/sba_iommu: return error code from sba_map_sg_attrs()
  MIPS/jazzdma: return error code from jazz_dma_map_sg()
  powerpc/iommu: return error code from .map_sg() ops
  s390/pci: return error code from s390_dma_map_sg()
  sparc/iommu: return error codes from .map_sg() ops
  parisc: return error code from .map_sg() ops
  xen: swiotlb: return error code from xen_swiotlb_map_sg()
  x86/amd_gart: return error code from gart_map_sg()
  dma-mapping: return error code from dma_dummy_map_sg()

 arch/alpha/kernel/pci_iommu.c   | 10 +++-
 arch/arm/mm/dma-mapping.c   | 22 +---
 arch/ia64/hp/common/sba_iommu.c |  9 +--
 arch/mips/jazz/jazzdma.c|  2 +-
 arch/powerpc/kernel/iommu.c |  4 +-
 arch/powerpc/platforms/ps3/system-bus.c |  2 +-
 arch/powerpc/platforms/pseries/vio.c|  5 +-
 arch/s390/pci/pci_dma.c | 12 ++--
 arch/sparc/kernel/iommu.c   |  4 +-
 arch/sparc/kernel/pci_sun4v.c   |  4 +-
 arch/sparc/mm/iommu.c   |  2 +-
 arch/x86/kernel/amd_gart_64.c   | 16 +++---
 drivers/iommu/dma-iommu.c   | 20 ---
 drivers/iommu/iommu.c   | 15 +++--
 drivers/parisc/ccio-dma.c   |  2 +-
 drivers/parisc/sba_iommu.c  |  2 +-
 drivers/xen/swiotlb-xen.c   |  2 +-
 include/linux/dma-map-ops.h |  6 +-
 include/linux/dma-mapping.h | 35 +++-
 include/linux/iommu.h   | 22 
 kernel/dma/direct.c |  2 +-
 kernel/dma/dummy.c  |  2 +-
 kernel/dma/mapping.c| 73 ++---
 23 files changed, 165 insertions(+), 108 deletions(-)


base-commit: e73f0f0ee7541171d89f2e2491130c7771ba58d3
--
2.20.1

[PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg()

[Logan Gunthorpe]

Pass through appropriate error codes from iommu_dma_map_sg() now that
the error code will be passed through dma_map_sgtable().

Signed-off-by: Logan Gunthorpe 
Cc: Joerg Roedel 
Cc: Will Deacon 
---
 drivers/iommu/dma-iommu.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..9d35e9994306 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -972,7 +972,7 @@ static int iommu_dma_map_sg_swiotlb(struct device *dev, 
struct scatterlist *sg,
 
 out_unmap:
iommu_dma_unmap_sg_swiotlb(dev, sg, i, dir, attrs | 
DMA_ATTR_SKIP_CPU_SYNC);
-   return 0;
+   return -EINVAL;
 }
 
 /*
@@ -993,11 +993,14 @@ static int iommu_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
dma_addr_t iova;
size_t iova_len = 0;
unsigned long mask = dma_get_seg_boundary(dev);
+   ssize_t ret;
int i;
 
-   if (static_branch_unlikely(_deferred_attach_enabled) &&
-   iommu_deferred_attach(dev, domain))
-   return 0;
+   if (static_branch_unlikely(_deferred_attach_enabled)) {
+   ret = iommu_deferred_attach(dev, domain);
+   if (ret)
+   return ret;
+   }
 
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
@@ -1045,14 +1048,17 @@ static int iommu_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
}
 
iova = iommu_dma_alloc_iova(domain, iova_len, dma_get_mask(dev), dev);
-   if (!iova)
+   if (!iova) {
+   ret = -ENOMEM;
goto out_restore_sg;
+   }
 
/*
 * We'll leave any physical concatenation to the IOMMU driver's
 * implementation - it knows better than we do.
 */
-   if (iommu_map_sg_atomic(domain, iova, sg, nents, prot) < iova_len)
+   ret = iommu_map_sg_atomic(domain, iova, sg, nents, prot);
+   if (ret < iova_len)
goto out_free_iova;
 
return __finalise_sg(dev, sg, nents, iova);
@@ -1061,7 +1067,7 @@ static int iommu_dma_map_sg(struct device *dev, struct 
scatterlist *sg,
iommu_dma_free_iova(cookie, iova, iova_len, NULL);
 out_restore_sg:
__invalidate_sg(sg, nents);
-   return 0;
+   return ret;
 }
 
 static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-- 
2.20.1

[PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes

[Logan Gunthorpe]

Allow dma_map_sgtable() to pass errors from the map_sg() ops. This
will be required for returning appropriate error codes when mapping
P2PDMA memory.

Introduce __dma_map_sg_attrs() which will return the raw error code
from the map_sg operation (whether it be negative or zero). Then add a
dma_map_sg_attrs() wrapper to convert any negative errors to zero to
satisfy the existing calling convention.

dma_map_sgtable() will convert a zero error return for old map_sg() ops
into a -EINVAL return and return any negative errors as reported.

This allows map_sg implementations to start returning multiple
negative error codes. Legacy map_sg implementations can continue
to return zero until they are all converted.

Signed-off-by: Logan Gunthorpe 
---
 include/linux/dma-map-ops.h |  8 +++-
 include/linux/dma-mapping.h | 35 --
 kernel/dma/mapping.c| 73 +
 3 files changed, 78 insertions(+), 38 deletions(-)

diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 0d53a96a3d64..eaa969be8284 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -41,8 +41,12 @@ struct dma_map_ops {
size_t size, enum dma_data_direction dir,
unsigned long attrs);
/*
-* map_sg returns 0 on error and a value > 0 on success.
-* It should never return a value < 0.
+* map_sg should return a negative error code on error.
+* dma_map_sgtable() will return the error code returned and convert
+* a zero return (for legacy implementations) into -EINVAL.
+*
+* dma_map_sg() will always return zero on any negative or zero
+* return to satisfy its own calling convention.
 */
int (*map_sg)(struct device *dev, struct scatterlist *sg, int nents,
enum dma_data_direction dir, unsigned long attrs);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 183e7103a66d..daa1e360f0ee 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -110,6 +110,8 @@ int dma_map_sg_attrs(struct device *dev, struct scatterlist 
*sg, int nents,
 void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
  int nents, enum dma_data_direction dir,
  unsigned long attrs);
+int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
+   enum dma_data_direction dir, unsigned long attrs);
 dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
size_t size, enum dma_data_direction dir, unsigned long attrs);
 void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
@@ -174,6 +176,11 @@ static inline void dma_unmap_sg_attrs(struct device *dev,
unsigned long attrs)
 {
 }
+static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
+   enum dma_data_direction dir, unsigned long attrs)
+{
+   return -EOPNOTSUPP;
+}
 static inline dma_addr_t dma_map_resource(struct device *dev,
phys_addr_t phys_addr, size_t size, enum dma_data_direction dir,
unsigned long attrs)
@@ -343,34 +350,6 @@ static inline void dma_sync_single_range_for_device(struct 
device *dev,
return dma_sync_single_for_device(dev, addr + offset, size, dir);
 }
 
-/**
- * dma_map_sgtable - Map the given buffer for DMA
- * @dev:   The device for which to perform the DMA operation
- * @sgt:   The sg_table object describing the buffer
- * @dir:   DMA direction
- * @attrs: Optional DMA attributes for the map operation
- *
- * Maps a buffer described by a scatterlist stored in the given sg_table
- * object for the @dir DMA operation by the @dev device. After success the
- * ownership for the buffer is transferred to the DMA domain.  One has to
- * call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
- * ownership of the buffer back to the CPU domain before touching the
- * buffer by the CPU.
- *
- * Returns 0 on success or -EINVAL on error during mapping the buffer.
- */
-static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
-   enum dma_data_direction dir, unsigned long attrs)
-{
-   int nents;
-
-   nents = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
-   if (nents <= 0)
-   return -EINVAL;
-   sgt->nents = nents;
-   return 0;
-}
-
 /**
  * dma_unmap_sgtable - Unmap the given buffer for DMA
  * @dev:   The device for which to perform the DMA operation
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 2b06a809d0b9..30f89d244566 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -177,12 +177,8 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t 
addr, size_t size,
 }
 EXPORT_SYMBOL(dma_unmap_page_attrs);
 
-/*
- * dma_maps_sg_attrs returns 0 on error and > 0 

[PATCH v1 02/16] dma-direct: Return appropriate error code from dma_direct_map_sg()

[Logan Gunthorpe]

Now that the map_sg() op expects error codes instead of return zero on
error, convert dma_direct_map_sg() to return an error code. The
only error to return presently is EINVAL if a page could not
be mapped.

Signed-off-by: Logan Gunthorpe 
---
 kernel/dma/direct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index f737e3347059..803ee9321170 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -411,7 +411,7 @@ int dma_direct_map_sg(struct device *dev, struct 
scatterlist *sgl, int nents,
 
 out_unmap:
dma_direct_unmap_sg(dev, sgl, i, dir, attrs | DMA_ATTR_SKIP_CPU_SYNC);
-   return 0;
+   return -EINVAL;
 }
 
 dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
-- 
2.20.1

[PATCH v1 03/16] iommu: Return full error code from iommu_map_sg[_atomic]()

[Logan Gunthorpe]

Convert to ssize_t return code so the return code from __iommu_map()
can be returned all the way down through dma_iommu_map_sg().

Signed-off-by: Logan Gunthorpe 
Cc: Joerg Roedel 
Cc: Will Deacon 
---
 drivers/iommu/iommu.c | 15 +++
 include/linux/iommu.h | 22 +++---
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..bf971b4e34aa 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2567,9 +2567,9 @@ size_t iommu_unmap_fast(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_unmap_fast);
 
-static size_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
-struct scatterlist *sg, unsigned int nents, int 
prot,
-gfp_t gfp)
+static ssize_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+   struct scatterlist *sg, unsigned int nents, int prot,
+   gfp_t gfp)
 {
const struct iommu_ops *ops = domain->ops;
size_t len = 0, mapped = 0;
@@ -2610,19 +2610,18 @@ static size_t __iommu_map_sg(struct iommu_domain 
*domain, unsigned long iova,
/* undo mappings already done */
iommu_unmap(domain, iova, mapped);
 
-   return 0;
-
+   return ret;
 }
 
-size_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
-   struct scatterlist *sg, unsigned int nents, int prot)
+ssize_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+struct scatterlist *sg, unsigned int nents, int prot)
 {
might_sleep();
return __iommu_map_sg(domain, iova, sg, nents, prot, GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(iommu_map_sg);
 
-size_t iommu_map_sg_atomic(struct iommu_domain *domain, unsigned long iova,
+ssize_t iommu_map_sg_atomic(struct iommu_domain *domain, unsigned long iova,
struct scatterlist *sg, unsigned int nents, int prot)
 {
return __iommu_map_sg(domain, iova, sg, nents, prot, GFP_ATOMIC);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..9369458ba1bd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -414,11 +414,11 @@ extern size_t iommu_unmap(struct iommu_domain *domain, 
unsigned long iova,
 extern size_t iommu_unmap_fast(struct iommu_domain *domain,
   unsigned long iova, size_t size,
   struct iommu_iotlb_gather *iotlb_gather);
-extern size_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
-  struct scatterlist *sg,unsigned int nents, int prot);
-extern size_t iommu_map_sg_atomic(struct iommu_domain *domain,
- unsigned long iova, struct scatterlist *sg,
- unsigned int nents, int prot);
+extern ssize_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+   struct scatterlist *sg, unsigned int nents, int prot);
+extern ssize_t iommu_map_sg_atomic(struct iommu_domain *domain,
+  unsigned long iova, struct scatterlist *sg,
+  unsigned int nents, int prot);
 extern phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t 
iova);
 extern void iommu_set_fault_handler(struct iommu_domain *domain,
iommu_fault_handler_t handler, void *token);
@@ -679,18 +679,18 @@ static inline size_t iommu_unmap_fast(struct iommu_domain 
*domain,
return 0;
 }
 
-static inline size_t iommu_map_sg(struct iommu_domain *domain,
- unsigned long iova, struct scatterlist *sg,
- unsigned int nents, int prot)
+static inline ssize_t iommu_map_sg(struct iommu_domain *domain,
+  unsigned long iova, struct scatterlist *sg,
+  unsigned int nents, int prot)
 {
-   return 0;
+   return -ENODEV;
 }
 
-static inline size_t iommu_map_sg_atomic(struct iommu_domain *domain,
+static inline ssize_t iommu_map_sg_atomic(struct iommu_domain *domain,
  unsigned long iova, struct scatterlist *sg,
  unsigned int nents, int prot)
 {
-   return 0;
+   return -ENODEV;
 }
 
 static inline void iommu_flush_iotlb_all(struct iommu_domain *domain)
-- 
2.20.1

Re: [PATCH for-4.13] x86/tsx: Fix backport of "x86/cpuid: Rework HLE and RTM handling"

[Andrew Cooper]

On 15/07/2021 16:39, Jan Beulich wrote:
> On 15.07.2021 17:10, Andrew Cooper wrote:
>> The backport dropped the hunk deleting the setup_clear_cpu_cap() for HLE/RTM,
>> but retained the hunk adding setup_force_cpu_cap().
>>
>> Calling both force and clear on the same feature elicits an error, and clear
>> takes precedence,
> Right, I particularly didn't pay attention to this interaction
> aspect.
>
>> which breaks the part of the bufix which makes migration
>> from older versions of Xen function safely for VMs using TSX.
>>
>> Fixes: f17d848c4caa ("x86/cpuid: Rework HLE and RTM handling")
>> Signed-off-by: Andrew Cooper 
> Acked-by: Jan Beulich 

Thanks.

> Are you going to put this in, or should I?

Feel free to.

~Andrew

[linux-linus test] 163706: regressions - FAIL

[osstest service owner]

flight 163706 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/163706/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-xsm7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-examine   6 xen-install  fail REGR. vs. 152332
 test-amd64-i386-libvirt   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-amd64-i386-xl7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-raw7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-pvshim 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-shadow 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-freebsd10-i386  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-amd64-coresched-i386-xl  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-libvirt-xsm   7 xen-install  fail REGR. vs. 152332
 test-amd64-amd64-libvirt-vhd 13 guest-start  fail REGR. vs. 152332
 test-arm64-arm64-xl-credit1  13 debian-fixup fail REGR. vs. 152332
 test-arm64-arm64-xl-xsm  13 debian-fixup fail REGR. vs. 152332
 test-arm64-arm64-xl-credit2  13 debian-fixup fail REGR. vs. 152332
 test-amd64-amd64-amd64-pvgrub 20 guest-stop  fail REGR. vs. 152332
 test-arm64-arm64-xl-thunderx 13 debian-fixup fail REGR. vs. 152332
 test-arm64-arm64-xl  13 debian-fixup fail REGR. vs. 152332
 test-amd64-amd64-i386-pvgrub 20 guest-stop   fail REGR. vs. 152332
 test-amd64-amd64-xl-qcow213 guest-start  fail REGR. vs. 152332
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 152332
 test-armhf-armhf-xl-vhd  13 guest-start  fail REGR. vs. 152332

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 152332
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 152332
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-check 

Re: [PATCH for-4.13] x86/tsx: Fix backport of "x86/cpuid: Rework HLE and RTM handling"

[Jan Beulich]

On 15.07.2021 17:10, Andrew Cooper wrote:
> The backport dropped the hunk deleting the setup_clear_cpu_cap() for HLE/RTM,
> but retained the hunk adding setup_force_cpu_cap().
> 
> Calling both force and clear on the same feature elicits an error, and clear
> takes precedence,

Right, I particularly didn't pay attention to this interaction
aspect.

> which breaks the part of the bufix which makes migration
> from older versions of Xen function safely for VMs using TSX.
> 
> Fixes: f17d848c4caa ("x86/cpuid: Rework HLE and RTM handling")
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 

Are you going to put this in, or should I?

Jan

Re: [PATCH] xen-blkfront: sanitize the removal state machine

[Jens Axboe]

On 7/15/21 8:17 AM, Christoph Hellwig wrote:
> xen-blkfront has a weird protocol where close message from the remote
> side can be delayed, and where hot removals are treated somewhat
> differently from regular removals, all leading to potential NULL
> pointer removals, and a del_gendisk from the block device release
> method, which will deadlock. Fix this by just performing normal hot
> removals even when the device is opened like all other Linux block
> drivers.

Applied, thanks.

-- 
Jens Axboe

Re: [PATCH v2 4/4] x86/debug: move domain_pause_for_debugger to debugger.c

[Jan Beulich]

On 14.07.2021 22:37, Bobby Eshleman wrote:
> The function domain_pause_for_debugger() is conditionally compiled if
> CONFIG_CRASH_DEBUG=y.  Instead of placing an extra #ifdef inside
> domain.c, this commit moves domain_pause_for_debugger() into
> x86/debugger.c which is only built by Kbuild given CONFIG_CRASH_DEBUG=y.
> 
> Signed-off-by: Bobby Eshleman 

I already gave

Acked-by: Jan Beulich 

on this patch. Please accumulate tags as you send new versions, unless
changes you make require you to drop them (and require them to be re-
offered after re-review).

Jan

Re: [PATCH v2 3/4] x86/debug: move debugger_trap_entry into debugger.c not inlined

[Jan Beulich]

On 14.07.2021 22:37, Bobby Eshleman wrote:
> The function debugger_trap_entry() is somewhat large for an inlined
> function.  This commit moves debugger_trap_entry() into debugger.c and
> makes it not inlined.
> 
> Signed-off-by: Bobby Eshleman 

Acked-by: Jan Beulich 

Re: [PATCH v2 2/4] build: use common stubs for debugger_trap_* functions if !CONFIG_CRASH_DEBUG

[Jan Beulich]

On 14.07.2021 22:37, Bobby Eshleman wrote:
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -16,6 +16,7 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 

I don't think this is needed here; instead I think ...

> @@ -41,7 +42,6 @@
>  #include 
>  #include 
>  #include 
> -#include 

... this wants to be done in patch 1 already.

> --- a/xen/common/keyhandler.c
> +++ b/xen/common/keyhandler.c
> @@ -3,6 +3,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -20,7 +21,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 

Not sure about this - as indicated maybe the code needing the include
instead wants to move to x86'es new debugger.c.

> --- /dev/null
> +++ b/xen/include/xen/debugger.h
> @@ -0,0 +1,69 @@
> +/**
> + * Generic hooks into arch-dependent Xen.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see .
> + *
> + * Each debugger should define three functions here:
> + *
> + * 1. debugger_trap_entry():
> + *  Called at start of any synchronous fault or trap, before any other work
> + *  is done. The idea is that if your debugger deliberately caused the trap
> + *  (e.g. to implement breakpoints or data watchpoints) then you can take
> + *  appropriate action and return a non-zero value to cause early exit from
> + *  the trap function.
> + *
> + * 2. debugger_trap_fatal():
> + *  Called when Xen is about to give up and crash. Typically you will use 
> this
> + *  hook to drop into a debug session. It can also be used to hook off
> + *  deliberately caused traps (which you then handle and return non-zero).
> + *
> + * 3. debugger_trap_immediate():
> + *  Called if we want to drop into a debugger now.  This is essentially the
> + *  same as debugger_trap_fatal, except that we use the current register 
> state
> + *  rather than the state which was in effect when we took the trap.
> + *  For example: if we're dying because of an unhandled exception, we call
> + *  debugger_trap_fatal; if we're dying because of a panic() we call
> + *  debugger_trap_immediate().
> + */
> +
> +#ifndef __XEN_DEBUGGER_H__
> +#define __XEN_DEBUGGER_H__
> +
> +#ifdef CONFIG_CRASH_DEBUG
> +
> +#include 
> +
> +#else
> +
> +#include 
> +#include 

I don't think you need either of these for the stubs below.

> +static inline void domain_pause_for_debugger(void)
> +{
> +}
> +
> +static inline bool debugger_trap_fatal(
> +unsigned int vector, const struct cpu_user_regs *regs)
> +{
> +return false;
> +}
> +
> +static inline void debugger_trap_immediate(void)
> +{
> +}
> +
> +static inline bool debugger_trap_entry(
> +unsigned int vector, const struct cpu_user_regs *regs)
> +{
> +return false;
> +}

Of these stubs, after patch 1 only debugger_trap_immediate() is needed
outside of arch/x86/. Perhaps everything else wants to remain in x86'es
per-arch header?

Jan

[PATCH for-4.13] x86/tsx: Fix backport of "x86/cpuid: Rework HLE and RTM handling"

[Andrew Cooper]

The backport dropped the hunk deleting the setup_clear_cpu_cap() for HLE/RTM,
but retained the hunk adding setup_force_cpu_cap().

Calling both force and clear on the same feature elicits an error, and clear
takes precedence, which breaks the part of the bufix which makes migration
from older versions of Xen function safely for VMs using TSX.

Fixes: f17d848c4caa ("x86/cpuid: Rework HLE and RTM handling")
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/spec_ctrl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 33343062a7b6..1cfd02d7d7cf 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -1150,9 +1150,6 @@ void __init init_speculation_mitigations(void)
  ((hw_smt_enabled && opt_smt) ||
   !boot_cpu_has(X86_FEATURE_SC_VERW_IDLE)) )
 {
-setup_clear_cpu_cap(X86_FEATURE_HLE);
-setup_clear_cpu_cap(X86_FEATURE_RTM);
-
 opt_tsx = 0;
 tsx_init();
 }
-- 
2.11.0

Re: [PATCH v2 4/4] bus: Make remove callback return void

[Thomas Bogendoerfer]

On Tue, Jul 06, 2021 at 05:48:03PM +0200, Uwe Kleine-König wrote:
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
> 
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
> 
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
> 
> Acked-by: Russell King (Oracle)  (For ARM, Amba 
> and related parts)
> Acked-by: Mark Brown 
> Acked-by: Chen-Yu Tsai  (for drivers/bus/sunxi-rsb.c)
> Acked-by: Pali Rohár 
> Acked-by: Mauro Carvalho Chehab  (for drivers/media)
> Acked-by: Hans de Goede  (For drivers/platform)
> Acked-by: Alexandre Belloni 
> Acked-By: Vinod Koul 
> Acked-by: Juergen Gross  (For Xen)
> Acked-by: Lee Jones  (For drivers/mfd)
> Acked-by: Johannes Thumshirn  (For drivers/mcb)
> Acked-by: Johan Hovold 
> Acked-by: Srinivas Kandagatla  (For 
> drivers/slimbus)
> Acked-by: Kirti Wankhede  (For drivers/vfio)
> Acked-by: Maximilian Luz 
> Acked-by: Heikki Krogerus  (For ulpi and 
> typec)
> Acked-by: Samuel Iglesias Gonsálvez  (For ipack)
> Reviewed-by: Tom Rix  (For fpga)
> Acked-by: Geoff Levand  (For ps3)
> Signed-off-by: Uwe Kleine-König 
> ---
> [...] 
>  arch/mips/sgi-ip22/ip22-gio.c | 3 +--

Acked-by: Thomas Bogendoerfer 

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.[ RFC1925, 2.3 ]

Re: preparations for 4.15.1 and 4.13.4

[Jan Beulich]

On 15.07.2021 16:11, Olaf Hering wrote:
> Am Thu, 15 Jul 2021 09:58:24 +0200
> schrieb Jan Beulich :
> 
>> Please point out backports you find missing from the respective staging 
>> branches, but which you consider relevant.
> 
> Depending on how green the CI is supposed to be:
> 
> 76416c459c libfsimage: fix parentheses in macro parameters
> e54c433adf libfsimage: fix clang 10 build

Ian, that's again something for you to consider.

Thanks, Jan

> This will likely turn the Leap clang builds at 
> https://gitlab.com/xen-project/xen/-/pipelines/337629824 green.
> 
> Olaf
> 

[PATCH] xen-blkfront: sanitize the removal state machine

[Christoph Hellwig]

xen-blkfront has a weird protocol where close message from the remote
side can be delayed, and where hot removals are treated somewhat
differently from regular removals, all leading to potential NULL
pointer removals, and a del_gendisk from the block device release
method, which will deadlock. Fix this by just performing normal hot
removals even when the device is opened like all other Linux block
drivers.

Fixes: c76f48eb5c08 ("block: take bd_mutex around delete_partitions in 
del_gendisk")
Reported-by: Vitaly Kuznetsov 
Signed-off-by: Christoph Hellwig 
Tested-by: Vitaly Kuznetsov 
---
 drivers/block/xen-blkfront.c | 224 ---
 1 file changed, 26 insertions(+), 198 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8d49f8fa98bb..d83fee21f6c5 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -502,34 +502,21 @@ static int blkif_getgeo(struct block_device *bd, struct 
hd_geometry *hg)
 static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
   unsigned command, unsigned long argument)
 {
-   struct blkfront_info *info = bdev->bd_disk->private_data;
int i;
 
-   dev_dbg(>xbdev->dev, "command: 0x%x, argument: 0x%lx\n",
-   command, (long)argument);
-
switch (command) {
case CDROMMULTISESSION:
-   dev_dbg(>xbdev->dev, "FIXME: support multisession CDs 
later\n");
for (i = 0; i < sizeof(struct cdrom_multisession); i++)
if (put_user(0, (char __user *)(argument + i)))
return -EFAULT;
return 0;
-
-   case CDROM_GET_CAPABILITY: {
-   struct gendisk *gd = info->gd;
-   if (gd->flags & GENHD_FL_CD)
+   case CDROM_GET_CAPABILITY:
+   if (bdev->bd_disk->flags & GENHD_FL_CD)
return 0;
return -EINVAL;
-   }
-
default:
-   /*printk(KERN_ALERT "ioctl %08x not supported by Xen blkdev\n",
- command);*/
-   return -EINVAL; /* same return as native Linux */
+   return -EINVAL;
}
-
-   return 0;
 }
 
 static unsigned long blkif_ring_get_request(struct blkfront_ring_info *rinfo,
@@ -1177,36 +1164,6 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
return err;
 }
 
-static void xlvbd_release_gendisk(struct blkfront_info *info)
-{
-   unsigned int minor, nr_minors, i;
-   struct blkfront_ring_info *rinfo;
-
-   if (info->rq == NULL)
-   return;
-
-   /* No more blkif_request(). */
-   blk_mq_stop_hw_queues(info->rq);
-
-   for_each_rinfo(info, rinfo, i) {
-   /* No more gnttab callback work. */
-   gnttab_cancel_free_callback(>callback);
-
-   /* Flush gnttab callback work. Must be done with no locks held. 
*/
-   flush_work(>work);
-   }
-
-   del_gendisk(info->gd);
-
-   minor = info->gd->first_minor;
-   nr_minors = info->gd->minors;
-   xlbd_release_minors(minor, nr_minors);
-
-   blk_cleanup_disk(info->gd);
-   info->gd = NULL;
-   blk_mq_free_tag_set(>tag_set);
-}
-
 /* Already hold rinfo->ring_lock. */
 static inline void kick_pending_request_queues_locked(struct 
blkfront_ring_info *rinfo)
 {
@@ -1756,12 +1713,6 @@ static int write_per_ring_nodes(struct 
xenbus_transaction xbt,
return err;
 }
 
-static void free_info(struct blkfront_info *info)
-{
-   list_del(>info_list);
-   kfree(info);
-}
-
 /* Common code used when first setting up, and when resuming. */
 static int talk_to_blkback(struct xenbus_device *dev,
   struct blkfront_info *info)
@@ -1880,13 +1831,6 @@ static int talk_to_blkback(struct xenbus_device *dev,
xenbus_dev_fatal(dev, err, "%s", message);
  destroy_blkring:
blkif_free(info, 0);
-
-   mutex_lock(_mutex);
-   free_info(info);
-   mutex_unlock(_mutex);
-
-   dev_set_drvdata(>dev, NULL);
-
return err;
 }
 
@@ -2126,38 +2070,26 @@ static int blkfront_resume(struct xenbus_device *dev)
 static void blkfront_closing(struct blkfront_info *info)
 {
struct xenbus_device *xbdev = info->xbdev;
-   struct block_device *bdev = NULL;
-
-   mutex_lock(>mutex);
+   struct blkfront_ring_info *rinfo;
+   unsigned int i;
 
-   if (xbdev->state == XenbusStateClosing) {
-   mutex_unlock(>mutex);
+   if (xbdev->state == XenbusStateClosing)
return;
-   }
 
-   if (info->gd)
-   bdev = bdgrab(info->gd->part0);
-
-   mutex_unlock(>mutex);
-
-   if (!bdev) {
-   xenbus_frontend_closed(xbdev);
-   return;
-   }
+   /* No more blkif_request(). */
+   blk_mq_stop_hw_queues(info->rq);
+   blk_set_queue_dying(info->rq);
+   set_capacity(info->gd, 0);
 
-   

Re: preparations for 4.15.1 and 4.13.4

[Olaf Hering]

Am Thu, 15 Jul 2021 09:58:24 +0200
schrieb Jan Beulich :

> Please point out backports you find missing from the respective staging 
> branches, but which you consider relevant.

Depending on how green the CI is supposed to be:

76416c459c libfsimage: fix parentheses in macro parameters
e54c433adf libfsimage: fix clang 10 build

This will likely turn the Leap clang builds at 
https://gitlab.com/xen-project/xen/-/pipelines/337629824 green.

Olaf


pgpFhInBX17f2.pgp
Description: Digitale Signatur von OpenPGP

Re: [BUG report] Deadlock in xen-blkfront upon device hot-unplug

[Vitaly Kuznetsov]

Christoph Hellwig  writes:

> On Thu, Jul 15, 2021 at 03:17:37PM +0200, Vitaly Kuznetsov wrote:
>> Christoph Hellwig  writes:
>> 
>> > On Thu, Jul 15, 2021 at 11:16:30AM +0200, Vitaly Kuznetsov wrote:
>> >> I'm observing a deadlock every time I try to unplug a xen-blkfront
>> >> device. With 5.14-rc1+ the deadlock looks like:
>> >
>> > I did actually stumble over this a few days ago just from code
>> > inspection.  Below is what I come up with, can you give it a spin?
>> 
>> This eliminates the deadlock, thanks! Unfortunately, this reveals the
>> same issue I observed when I just dropped taking the mutex from
>> blkfront_closing():
>
> Yeah, this still left too much cruft in blkfront_closing.  Can you
> try this version instead?
>

This one seems to work fine for me! Feel free to throw

Tested-by: Vitaly Kuznetsov 

in. Thanks a lot!

-- 
Vitaly

Re: [BUG report] Deadlock in xen-blkfront upon device hot-unplug

[Christoph Hellwig]

On Thu, Jul 15, 2021 at 03:17:37PM +0200, Vitaly Kuznetsov wrote:
> Christoph Hellwig  writes:
> 
> > On Thu, Jul 15, 2021 at 11:16:30AM +0200, Vitaly Kuznetsov wrote:
> >> I'm observing a deadlock every time I try to unplug a xen-blkfront
> >> device. With 5.14-rc1+ the deadlock looks like:
> >
> > I did actually stumble over this a few days ago just from code
> > inspection.  Below is what I come up with, can you give it a spin?
> 
> This eliminates the deadlock, thanks! Unfortunately, this reveals the
> same issue I observed when I just dropped taking the mutex from
> blkfront_closing():

Yeah, this still left too much cruft in blkfront_closing.  Can you
try this version instead?

---
>From 4d926a44f15d2051909132182345754df23cc13d Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Tue, 29 Jun 2021 13:49:22 +0200
Subject: xen-blkfront: sanitize the removal state machine

xen-blkfront has a weird protocol where close message from the remote
side can be delayed, and where hot removals are treated somewhat
differently from regular removals, all leading to potential NULL
pointer removals.  Fix this by just performing normal hot removals
even when the device is opened like all other Linux block drivers.

Signed-off-by: Christoph Hellwig 
---
 drivers/block/xen-blkfront.c | 208 ---
 1 file changed, 24 insertions(+), 184 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8d49f8fa98bb..09ceb7bd585a 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -502,34 +502,21 @@ static int blkif_getgeo(struct block_device *bd, struct 
hd_geometry *hg)
 static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
   unsigned command, unsigned long argument)
 {
-   struct blkfront_info *info = bdev->bd_disk->private_data;
int i;
 
-   dev_dbg(>xbdev->dev, "command: 0x%x, argument: 0x%lx\n",
-   command, (long)argument);
-
switch (command) {
case CDROMMULTISESSION:
-   dev_dbg(>xbdev->dev, "FIXME: support multisession CDs 
later\n");
for (i = 0; i < sizeof(struct cdrom_multisession); i++)
if (put_user(0, (char __user *)(argument + i)))
return -EFAULT;
return 0;
-
-   case CDROM_GET_CAPABILITY: {
-   struct gendisk *gd = info->gd;
-   if (gd->flags & GENHD_FL_CD)
+   case CDROM_GET_CAPABILITY:
+   if (bdev->bd_disk->flags & GENHD_FL_CD)
return 0;
return -EINVAL;
-   }
-
default:
-   /*printk(KERN_ALERT "ioctl %08x not supported by Xen blkdev\n",
- command);*/
-   return -EINVAL; /* same return as native Linux */
+   return -EINVAL;
}
-
-   return 0;
 }
 
 static unsigned long blkif_ring_get_request(struct blkfront_ring_info *rinfo,
@@ -1177,36 +1164,6 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
return err;
 }
 
-static void xlvbd_release_gendisk(struct blkfront_info *info)
-{
-   unsigned int minor, nr_minors, i;
-   struct blkfront_ring_info *rinfo;
-
-   if (info->rq == NULL)
-   return;
-
-   /* No more blkif_request(). */
-   blk_mq_stop_hw_queues(info->rq);
-
-   for_each_rinfo(info, rinfo, i) {
-   /* No more gnttab callback work. */
-   gnttab_cancel_free_callback(>callback);
-
-   /* Flush gnttab callback work. Must be done with no locks held. 
*/
-   flush_work(>work);
-   }
-
-   del_gendisk(info->gd);
-
-   minor = info->gd->first_minor;
-   nr_minors = info->gd->minors;
-   xlbd_release_minors(minor, nr_minors);
-
-   blk_cleanup_disk(info->gd);
-   info->gd = NULL;
-   blk_mq_free_tag_set(>tag_set);
-}
-
 /* Already hold rinfo->ring_lock. */
 static inline void kick_pending_request_queues_locked(struct 
blkfront_ring_info *rinfo)
 {
@@ -2126,38 +2083,26 @@ static int blkfront_resume(struct xenbus_device *dev)
 static void blkfront_closing(struct blkfront_info *info)
 {
struct xenbus_device *xbdev = info->xbdev;
-   struct block_device *bdev = NULL;
-
-   mutex_lock(>mutex);
+   struct blkfront_ring_info *rinfo;
+   unsigned int i;
 
-   if (xbdev->state == XenbusStateClosing) {
-   mutex_unlock(>mutex);
+   if (xbdev->state == XenbusStateClosing)
return;
-   }
-
-   if (info->gd)
-   bdev = bdgrab(info->gd->part0);
 
-   mutex_unlock(>mutex);
-
-   if (!bdev) {
-   xenbus_frontend_closed(xbdev);
-   return;
-   }
+   /* No more blkif_request(). */
+   blk_mq_stop_hw_queues(info->rq);
+   blk_set_queue_dying(info->rq);
+   set_capacity(info->gd, 0);
 
-   mutex_lock(>bd_disk->open_mutex);
+   for_each_rinfo(info, 

Re: [BUG report] Deadlock in xen-blkfront upon device hot-unplug

[Vitaly Kuznetsov]

Christoph Hellwig  writes:

> On Thu, Jul 15, 2021 at 11:16:30AM +0200, Vitaly Kuznetsov wrote:
>> I'm observing a deadlock every time I try to unplug a xen-blkfront
>> device. With 5.14-rc1+ the deadlock looks like:
>
> I did actually stumble over this a few days ago just from code
> inspection.  Below is what I come up with, can you give it a spin?

This eliminates the deadlock, thanks! Unfortunately, this reveals the
same issue I observed when I just dropped taking the mutex from
blkfront_closing():

[   66.455122] general protection fault, probably for non-canonical address 
0xf1af5e354e6da159:  [#1] SMP PTI
[   66.462802] CPU: 4 PID: 145 Comm: xenwatch Not tainted 5.14.0-rc1+ #370
[   66.467486] Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
[   66.472817] RIP: 0010:del_timer+0x1f/0x80
[   66.476570] Code: 71 af a3 00 eb c1 31 c0 c3 66 90 0f 1f 44 00 00 41 55 41 
54 45 31 e4 55 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 44 24 08 <48> 8b 47 
08 48 85 c0 74 2d 48 89 e6 48 89 fd e8 dd e8 ff ff 48 89
[   66.493967] RSP: 0018:b5c10426bcd8 EFLAGS: 00010086
[   66.499416] RAX: a49b3c9544841100 RBX: f1af5e354e6da101 RCX: 5ebf
[   66.506386] RDX: 5ec0 RSI: 0001 RDI: f1af5e354e6da151
[   66.512799] RBP: b5c10426bd30 R08: 0001 R09: 0001
[   66.518372] R10: 0001 R11: 0002 R12: 
[   66.523681] R13: 9aba8df63e40 R14:  R15: 9aba86f4
[   66.529365] FS:  () GS:9af60920() 
knlGS:
[   66.536187] CS:  0010 DS:  ES:  CR0: 80050033
[   66.540806] CR2: 7ff024600130 CR3: 00010117a006 CR4: 001706e0
[   66.546345] DR0:  DR1:  DR2: 
[   66.552322] DR3:  DR6: fffe0ff0 DR7: 0400
[   66.558501] Call Trace:
[   66.561449]  try_to_grab_pending+0x13f/0x2e0
[   66.565658]  cancel_delayed_work+0x2e/0xd0
[   66.570012]  blk_mq_stop_hw_queues+0x2d/0x50
[   66.574110]  blkfront_remove+0x30/0x130 [xen_blkfront]
[   66.579049]  xenbus_dev_remove+0x6d/0xf0
[   66.582473]  __device_release_driver+0x180/0x240
[   66.586963]  device_release_driver+0x26/0x40
[   66.591050]  bus_remove_device+0xef/0x160
[   66.594805]  device_del+0x18c/0x3e0
[   66.598570]  ? xenbus_probe_devices+0x120/0x120
[   66.602987]  ? klist_iter_exit+0x14/0x20
[   66.606915]  device_unregister+0x13/0x60
[   66.611135]  xenbus_dev_changed+0x174/0x1e0
[   66.615104]  xenwatch_thread+0x94/0x190
[   66.619028]  ? do_wait_intr_irq+0xb0/0xb0
[   66.623052]  ? xenbus_dev_request_and_reply+0x90/0x90
[   66.628218]  kthread+0x149/0x170
[   66.631509]  ? set_kthread_struct+0x40/0x40
[   66.635355]  ret_from_fork+0x22/0x30
[   66.639162] Modules linked in: vfat fat i2c_piix4 xfs libcrc32c 
crct10dif_pclmul crc32_pclmul crc32c_intel xen_blkfront ghash_clmulni_intel ena
[   66.650868] ---[ end trace 7fa9da1e39697767 ]---
[   66.655490] RIP: 0010:del_timer+0x1f/0x80
[   66.659813] Code: 71 af a3 00 eb c1 31 c0 c3 66 90 0f 1f 44 00 00 41 55 41 
54 45 31 e4 55 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 44 24 08 <48> 8b 47 
08 48 85 c0 74 2d 48 89 e6 48 89 fd e8 dd e8 ff ff 48 89
[   66.681045] RSP: 0018:b5c10426bcd8 EFLAGS: 00010086
[   66.685888] RAX: a49b3c9544841100 RBX: f1af5e354e6da101 RCX: 5ebf
[   66.692153] RDX: 5ec0 RSI: 0001 RDI: f1af5e354e6da151
[   66.698778] RBP: b5c10426bd30 R08: 0001 R09: 0001
[   66.705175] R10: 0001 R11: 0002 R12: 
[   66.711696] R13: 9aba8df63e40 R14:  R15: 9aba86f4
[   66.718035] FS:  () GS:9af60920() 
knlGS:
[   66.725210] CS:  0010 DS:  ES:  CR0: 80050033
[   66.730291] CR2: 7ff024600130 CR3: 00010117a006 CR4: 001706e0
[   66.736235] DR0:  DR1:  DR2: 
[   66.742373] DR3:  DR6: fffe0ff0 DR7: 0400
[   66.749026] BUG: sleeping function called from invalid context at 
include/linux/percpu-rwsem.h:49
[   66.756118] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 145, 
name: xenwatch
[   66.763473] INFO: lockdep is turned off.
[   66.767428] irq event stamp: 24256
[   66.770900] hardirqs last  enabled at (24255): [] 
_raw_spin_unlock_irqrestore+0x4b/0x5d
[   66.779620] hardirqs last disabled at (24256): [] 
try_to_grab_pending+0x15c/0x2e0
[   66.787763] softirqs last  enabled at (24196): [] 
__irq_exit_rcu+0xe1/0x100
[   66.794519] softirqs last disabled at (24191): [] 
__irq_exit_rcu+0xe1/0x100
[   66.801953] CPU: 4 PID: 145 Comm: xenwatch Tainted: G  D   
5.14.0-rc1+ #370
[   66.809315] Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
[   66.814924] Call Trace:
[   66.817461]  dump_stack_lvl+0x6a/0x9a
[   66.821171]  

Re: [BUG report] Deadlock in xen-blkfront upon device hot-unplug

[Christoph Hellwig]

On Thu, Jul 15, 2021 at 11:16:30AM +0200, Vitaly Kuznetsov wrote:
> I'm observing a deadlock every time I try to unplug a xen-blkfront
> device. With 5.14-rc1+ the deadlock looks like:

I did actually stumble over this a few days ago just from code
inspection.  Below is what I come up with, can you give it a spin?

---
>From 4fef3b917af51153dd99a958ee9064f0de3a8b6a Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Tue, 29 Jun 2021 13:49:22 +0200
Subject: xen-blkfront: sanitize the removal state machine

xen-blkfront has a weird protocol where close message from the remote
side can be delayed, and where hot removals are treated somewhat
differently from regular removals, all leading to potential NULL
pointer removals.  Fix this by just performing normal hot removals
even when the device is opened like all other Linux block drivers.

Signed-off-by: Christoph Hellwig 
---
 drivers/block/xen-blkfront.c | 182 +++
 1 file changed, 16 insertions(+), 166 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8d49f8fa98bb..0ba37c97aabd 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -502,34 +502,21 @@ static int blkif_getgeo(struct block_device *bd, struct 
hd_geometry *hg)
 static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
   unsigned command, unsigned long argument)
 {
-   struct blkfront_info *info = bdev->bd_disk->private_data;
int i;
 
-   dev_dbg(>xbdev->dev, "command: 0x%x, argument: 0x%lx\n",
-   command, (long)argument);
-
switch (command) {
case CDROMMULTISESSION:
-   dev_dbg(>xbdev->dev, "FIXME: support multisession CDs 
later\n");
for (i = 0; i < sizeof(struct cdrom_multisession); i++)
if (put_user(0, (char __user *)(argument + i)))
return -EFAULT;
return 0;
-
-   case CDROM_GET_CAPABILITY: {
-   struct gendisk *gd = info->gd;
-   if (gd->flags & GENHD_FL_CD)
+   case CDROM_GET_CAPABILITY:
+   if (bdev->bd_disk->flags & GENHD_FL_CD)
return 0;
return -EINVAL;
-   }
-
default:
-   /*printk(KERN_ALERT "ioctl %08x not supported by Xen blkdev\n",
- command);*/
-   return -EINVAL; /* same return as native Linux */
+   return -EINVAL;
}
-
-   return 0;
 }
 
 static unsigned long blkif_ring_get_request(struct blkfront_ring_info *rinfo,
@@ -1179,11 +1166,8 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
 
 static void xlvbd_release_gendisk(struct blkfront_info *info)
 {
-   unsigned int minor, nr_minors, i;
struct blkfront_ring_info *rinfo;
-
-   if (info->rq == NULL)
-   return;
+   unsigned int i;
 
/* No more blkif_request(). */
blk_mq_stop_hw_queues(info->rq);
@@ -1198,12 +1182,8 @@ static void xlvbd_release_gendisk(struct blkfront_info 
*info)
 
del_gendisk(info->gd);
 
-   minor = info->gd->first_minor;
-   nr_minors = info->gd->minors;
-   xlbd_release_minors(minor, nr_minors);
-
+   xlbd_release_minors(info->gd->first_minor, info->gd->minors);
blk_cleanup_disk(info->gd);
-   info->gd = NULL;
blk_mq_free_tag_set(>tag_set);
 }
 
@@ -2126,38 +2106,16 @@ static int blkfront_resume(struct xenbus_device *dev)
 static void blkfront_closing(struct blkfront_info *info)
 {
struct xenbus_device *xbdev = info->xbdev;
-   struct block_device *bdev = NULL;
-
-   mutex_lock(>mutex);
-
-   if (xbdev->state == XenbusStateClosing) {
-   mutex_unlock(>mutex);
-   return;
-   }
 
-   if (info->gd)
-   bdev = bdgrab(info->gd->part0);
-
-   mutex_unlock(>mutex);
-
-   if (!bdev) {
-   xenbus_frontend_closed(xbdev);
+   if (xbdev->state == XenbusStateClosing)
return;
-   }
 
-   mutex_lock(>bd_disk->open_mutex);
-
-   if (bdev->bd_openers) {
-   xenbus_dev_error(xbdev, -EBUSY,
-"Device in use; refusing to close");
-   xenbus_switch_state(xbdev, XenbusStateClosing);
-   } else {
+   mutex_lock(_mutex);
+   if (info->gd->flags & GENHD_FL_UP)
xlvbd_release_gendisk(info);
-   xenbus_frontend_closed(xbdev);
-   }
-
-   mutex_unlock(>bd_disk->open_mutex);
-   bdput(bdev);
+   mutex_unlock(_mutex);
+   
+   xenbus_frontend_closed(xbdev);
 }
 
 static void blkfront_setup_discard(struct blkfront_info *info)
@@ -2472,8 +2430,7 @@ static void blkback_changed(struct xenbus_device *dev,
break;
fallthrough;
case XenbusStateClosing:
-   if (info)
-   blkfront_closing(info);
+   

[qemu-mainline test] 163694: regressions - FAIL

[osstest service owner]

flight 163694 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/163694/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-freebsd11-amd64 13 guest-startfail REGR. vs. 163321
 test-amd64-i386-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 163321
 test-amd64-amd64-qemuu-freebsd12-amd64 13 guest-startfail REGR. vs. 163321
 test-amd64-i386-qemuu-rhel6hvm-amd 12 redhat-install fail REGR. vs. 163321
 test-amd64-amd64-libvirt-xsm 14 guest-start  fail REGR. vs. 163321
 test-amd64-i386-freebsd10-i386 13 guest-startfail REGR. vs. 163321
 test-amd64-amd64-libvirt 14 guest-start  fail REGR. vs. 163321
 test-amd64-i386-freebsd10-amd64 13 guest-start   fail REGR. vs. 163321
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 12 debian-hvm-install fail 
REGR. vs. 163321
 test-amd64-i386-libvirt-xsm  14 guest-start  fail REGR. vs. 163321
 test-amd64-i386-libvirt  14 guest-start  fail REGR. vs. 163321
 test-amd64-amd64-libvirt-pair 25 guest-start/debian  fail REGR. vs. 163321
 test-amd64-i386-libvirt-pair 25 guest-start/debian   fail REGR. vs. 163321
 test-amd64-amd64-qemuu-nested-amd 12 debian-hvm-install  fail REGR. vs. 163321
 test-amd64-i386-qemuu-rhel6hvm-intel 12 redhat-install   fail REGR. vs. 163321
 test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install  fail REGR. vs. 163321
 test-amd64-amd64-qemuu-nested-intel 12 debian-hvm-install fail REGR. vs. 163321
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 12 debian-hvm-install fail REGR. vs. 
163321
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 12 debian-hvm-install fail 
REGR. vs. 163321
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 12 debian-hvm-install fail 
REGR. vs. 163321
 test-arm64-arm64-libvirt-xsm 14 guest-start  fail REGR. vs. 163321
 test-amd64-i386-xl-qemuu-debianhvm-amd64 12 debian-hvm-install fail REGR. vs. 
163321
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail REGR. vs. 163321
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 12 debian-hvm-install fail 
REGR. vs. 163321
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail REGR. vs. 163321
 test-amd64-amd64-xl-qcow212 debian-di-installfail REGR. vs. 163321
 test-amd64-amd64-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 
163321
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 163321
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 163321
 test-amd64-amd64-libvirt-vhd 12 debian-di-installfail REGR. vs. 163321
 test-amd64-i386-xl-qemuu-win7-amd64 12 windows-install   fail REGR. vs. 163321
 test-armhf-armhf-xl-vhd  12 debian-di-installfail REGR. vs. 163321
 test-armhf-armhf-libvirt 14 guest-start  fail REGR. vs. 163321
 test-amd64-i386-xl-qemuu-ws16-amd64 12 windows-install   fail REGR. vs. 163321
 test-amd64-amd64-xl-qemuu-ws16-amd64 12 windows-install  fail REGR. vs. 163321
 test-armhf-armhf-libvirt-raw 12 debian-di-installfail REGR. vs. 163321

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 

Re: [PATCH v2 13/13] SUPPORT.md: write down restriction of 32-bit tool stacks

[Jan Beulich]

On 15.07.2021 11:05, Julien Grall wrote:
> On 15/07/2021 07:38, Jan Beulich wrote:
>> On 14.07.2021 20:16, Julien Grall wrote:
>>> On 05/07/2021 16:18, Jan Beulich wrote:
 Let's try to avoid giving the impression that 32-bit tool stacks are as
 capable as 64-bit ones.
>>>
>>> Would you be able to provide a few examples of the known issues in the
>>> commit message? This would be helpful for anyone to understand why we
>>> decided to drop the support.
>>
>> Not sure how useful this is going to be.
> 
> It would at least be useful to me, so I can make an informed decision. I 
> suspect it would also be for anyone reading it in the future. This is 
> rather frustrating to find commit message with barely any rationale and 
> no-one remembering why this was done...

Well, I've added "There are a number of cases there where 32-bit
types are used to hold e.g. frame numbers." Not sure whether you
consider this sufficient.

Problematic code may be primarily in areas Arm doesn't
care about (yet), like PCI pass-through or migration. But see e.g.
- xc_map_foreign_range()'s "mfn" and "size" parameters,
- xc_maximum_ram_page()'s "max_mfn" parameter,
- libxl_dom.c:hvm_build_set_params()'s "store_mfn" and "console_mfn"
  parameters,
- xs_introduce_domain()'s "mfn" parameter,
and quite a few more in particular in libxenguest.

And then there are also subtle oddities like xc_set_mem_access_multi()
having

xen_mem_access_op_t mao =
{
.op   = XENMEM_access_op_set_access_multi,
.domid= domain_id,
.access   = XENMEM_access_default + 1, /* Invalid value */
.pfn  = ~0UL, /* Invalid GFN */
.nr   = nr,
};

Clearly ~0UL won't have the intended effect even for 32-bit guests,
when the field is uint64_t (we get away here because for whatever
reason the hypervisor doesn't check that the field indeed is ~0UL).
But I wouldn't be surprised to find uses where there would be a
difference. One of the main aspects certainly is ...

> I vaguely recall a discussion about 64-bit hypercall ([1]). I assume the 
> decision to drop support is related to it, but I have no way to prove it 
> from the commit message.

... this. Some XENMEM_* may return 64-bit values, yet the hypercall
interface is limited to "long" return types. Not even the multicall
approach taken to work around the restriction to "int" would help
here for x86-32, as struct multicall_entry also uses xen_ulong_t
for its "result" field.

> It is also not clear why adding the restriction is the way to go...
> 
>> This would be pointing at the
>> declarations / definitions of various tool stack internal variables or
>> structure fields. Which also is why ...
> 
> ... is this because such issues are too widespread in libxc/libxl to fix 
> it in long term?

Fixing is an option, but until it gets fixed (if anyone really cared
to do so), spelling out the restriction looks to be an appropriate
step to me (or else I wouldn't have followed the request and created
this patch). Once suitably audited, fixed, and tested, I wouldn't see
a reason not to remove the restriction again.

Jan

Re: preparations for 4.15.1 and 4.13.4

[Jan Beulich]

On 15.07.2021 12:58, Andrew Cooper wrote:
> On 15/07/2021 08:58, Jan Beulich wrote:
>> All,
>>
>> the releases are due in a couple of weeks time (and 4.14.3 is
>> supposed to follow another few weeks later). Please point out backports
>> you find missing from the respective staging branches, but which you
>> consider relevant.
> 
> I've got a queue of:
> 
> * 429b0a5c62b9 - (HEAD -> staging-4.15) tools/libxenstat: fix populating
> vbd.rd_sect 
> * 41f0903e1632 - tools/python: fix Python3.4 TypeError in format string
> 
> * 67f798942caf - tools/python: handle libxl__physmap_info.name properly
> in convert-legacy-stream (74 seconds ago) 
> * e9709a83490f - tools: use integer division in
> convert-legacy-stream
> * 1a6824957d05 - (upstream/staging-4.15, origin/staging-4.15) build:
> clean "lib.a" 

You'll notice that this last one is already there.

Jan

Re: preparations for 4.15.1 and 4.13.4

[Andrew Cooper]

On 15/07/2021 08:58, Jan Beulich wrote:
> All,
>
> the releases are due in a couple of weeks time (and 4.14.3 is
> supposed to follow another few weeks later). Please point out backports
> you find missing from the respective staging branches, but which you
> consider relevant.

I've got a queue of:

* 429b0a5c62b9 - (HEAD -> staging-4.15) tools/libxenstat: fix populating
vbd.rd_sect 
* 41f0903e1632 - tools/python: fix Python3.4 TypeError in format string

* 67f798942caf - tools/python: handle libxl__physmap_info.name properly
in convert-legacy-stream (74 seconds ago) 
* e9709a83490f - tools: use integer division in
convert-legacy-stream
* 1a6824957d05 - (upstream/staging-4.15, origin/staging-4.15) build:
clean "lib.a" 

which I'd already ok'd with Ian as ok for backport.  I'll sort those out
on all applicable branches right now.

~Andrew

[libvirt test] 163704: regressions - FAIL

[osstest service owner]

flight 163704 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/163704/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  3c18bc304ecbf4ce7f02485a9198b849eb06cce7
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  370 days
Failing since151818  2020-07-11 04:18:52 Z  369 days  361 attempts
Testing same since   163704  2021-07-15 04:18:51 Z0 days1 attempts


People who touched revisions under test:
Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Aleksei Zakharov 
  Andika Triwidada 
  Andrea Bolognani 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastian Germann 
  Bastien Orivel 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Bjoern Walk 
  Boris Fiuczynski 
  Brian Turek 
  Bruno Haible 
  Chris Mayo 
  Christian Ehrhardt 
  Christian Schoenebeck 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Cédric Bosdonnat 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  Didik Supriadi 
  Dmytro Linkin 
  Eiichi Tsukata 
  Eric Farman 
  Erik Skultety 
  Fabian Affolter 
  Fabian Freyer 
  Fabiano Fidêncio 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Hela Basa 
  Helmut Grohne 
  Ian Wienand 
  Jakob Meng 
  Jamie Strandboge 
  Jamie Strandboge 
  Jan Kuparinen 
  Jean-Baptiste Holcroft 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jinsheng Zhang 
  Jiri Denemark 
  John Ferlan 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Kristina Hanicova 
  Laine Stump 
  Laszlo Ersek 
  Lee Yarwood 
  Liao Pingfang 
  Lin Ma 
  Lin Ma 
  Lin Ma 
  Liu Yiding 
  Luke Yue 
  Luyao Zhong 
  Marc Hartmayer 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Markus Schade 
  Martin Kletzander 
  Masayoshi Mizuma 
  Matt Coleman 
  Matt Coleman 
  Mauro Matteo Cascella 
  Meina Li 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  Moshe Levi 
  Muha Aliss 
  Nathan 
  Neal Gompa 
  Nick Shyrokovskiy 
  Nickys Music Group 
  Nico Pache 
  Nikolay Shirokovskiy 
  Olaf Hering 
  Olesya Gerasimenko 
  Orion Poplawski 
  Pany 
  Patrick Magauran 
  Paulo de Rezende Pinatti 
  Pavel Hrdina 
  Peng Liang 
  Peter Krempa 
  Pino Toscano 
  Pino Toscano 
  Piotr Drąg 
  Prathamesh Chavan 
  Ricky Tigg 
  Roman Bogorodskiy 
  Roman Bolshakov 
  Ryan Gahagan 
  Ryan Schmidt 
  Sam Hartman 
  Scott Shambarger 
  Sebastian Mitterle 
  SeongHyun Jo 
  Shalini Chellathurai Saroja 
  Shaojun Yang 
  Shi Lei 
  simmon 
  Simon Chopin 
  Simon Gaiser 
  Stefan Bader 
  Stefan Berger 
  Stefan Berger 
  Stefan Hajnoczi 
  Szymon Scholz 
  Thomas Huth 
  Tim Wiederhake 
  Tomáš Golembiovský 
  Tomáš Janoušek 
  Tuguoyi 
  Ville Skyttä 
  Vinayak Kale 
  Wang Xin 
  WangJian 
  Weblate 
  Wei Liu 
  Wei Liu 
  William Douglas 
  Yalei Li <274268...@qq.com>
  Yalei Li 
  Yang Hang 
  Yanqiu Zhang 
  Yaroslav Kargin 
  Yi Li 
  Yi Wang 
  Yuri Chornoivan 
  Zbigniew Jędrzejewski-Szmek 
  zhangjl02 
  Zheng Chuan 
  zhenwei pi 
  Zhenyu Zheng 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass

[BUG report] Deadlock in xen-blkfront upon device hot-unplug

[Vitaly Kuznetsov]

I'm observing a deadlock every time I try to unplug a xen-blkfront
device. With 5.14-rc1+ the deadlock looks like:

[  513.682405] 
[  513.686617] WARNING: possible recursive locking detected
[  513.691020] 5.14.0-rc1+ #370 Not tainted
[  513.694272] 
[  513.698528] xenwatch/144 is trying to acquire lock:
[  513.702424] 96dc4a4c1d28 (>open_mutex){+.+.}-{3:3}, at: 
del_gendisk+0x53/0x210
[  513.708768] 
   but task is already holding lock:
[  513.713320] 96dc4a4c1d28 (>open_mutex){+.+.}-{3:3}, at: 
blkback_changed+0x118/0xeb9 [xen_blkfront]
[  513.720369] 
   other info that might help us debug this:
[  513.724901]  Possible unsafe locking scenario:

[  513.729241]CPU0
[  513.731326]
[  513.733404]   lock(>open_mutex);
[  513.736679]   lock(>open_mutex);
[  513.739988] 
*** DEADLOCK ***

[  513.745524]  May be due to missing lock nesting notation

[  513.751438] 2 locks held by xenwatch/144:
[  513.755344]  #0: 8c9f3c70 (xenwatch_mutex){+.+.}-{3:3}, at: 
xenwatch_thread+0xe6/0x190
[  513.762137]  #1: 96dc4a4c1d28 (>open_mutex){+.+.}-{3:3}, at: 
blkback_changed+0x118/0xeb9 [xen_blkfront]
[  513.770381] 
   stack backtrace:
[  513.774785] CPU: 1 PID: 144 Comm: xenwatch Not tainted 5.14.0-rc1+ #370
[  513.780131] Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
[  513.785097] Call Trace:
[  513.787920]  dump_stack_lvl+0x6a/0x9a
[  513.791223]  __lock_acquire.cold+0x14a/0x2ba
[  513.794918]  ? mark_held_locks+0x50/0x80
[  513.798453]  lock_acquire+0xd3/0x2f0
[  513.801819]  ? del_gendisk+0x53/0x210
[  513.805334]  ? kernfs_put.part.0+0xe8/0x1b0
[  513.808905]  ? del_gendisk+0x53/0x210
[  513.812230]  __mutex_lock+0x8d/0x8c0
[  513.815415]  ? del_gendisk+0x53/0x210
[  513.818931]  ? kernfs_put.part.0+0xe8/0x1b0
[  513.822594]  del_gendisk+0x53/0x210
[  513.825782]  xlvbd_release_gendisk+0x6f/0xb0 [xen_blkfront]
[  513.830186]  blkback_changed+0x20e/0xeb9 [xen_blkfront]
[  513.834458]  ? xenbus_read_driver_state+0x39/0x60
[  513.838540]  xenwatch_thread+0x94/0x190
[  513.841936]  ? do_wait_intr_irq+0xb0/0xb0
[  513.845451]  ? xenbus_dev_request_and_reply+0x90/0x90
[  513.849885]  kthread+0x149/0x170
[  513.853039]  ? set_kthread_struct+0x40/0x40
[  513.857027]  ret_from_fork+0x22/0x30

My suspicion is that the problem was introduced by:

commit c76f48eb5c084b1e15c931ae8cc1826cd771d70d
Author: Christoph Hellwig 
Date:   Tue Apr 6 08:22:56 2021 +0200

block: take bd_mutex around delete_partitions in del_gendisk

blkfront_closing() takes '>bd_disk->open_mutex' around
xlvbd_release_gendisk() call which in its turn calls del_gendisk() which
after the above mentioned commit tries to take the same mutex. I may be
completely wrong though.

If I try to avoid taking the mutex from blkfront_closing(): 

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8d49f8fa98bb..9af6831492d4 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2145,8 +2145,6 @@ static void blkfront_closing(struct blkfront_info *info)
return;
}
 
-   mutex_lock(>bd_disk->open_mutex);
-
if (bdev->bd_openers) {
xenbus_dev_error(xbdev, -EBUSY,
 "Device in use; refusing to close");
@@ -2156,7 +2154,6 @@ static void blkfront_closing(struct blkfront_info *info)
xenbus_frontend_closed(xbdev);
}
 
-   mutex_unlock(>bd_disk->open_mutex);
bdput(bdev);
 }
 
the situation becomes even worse:

[   74.371465] general protection fault, probably for non-canonical address 
0xb0fa8ce8ee8a2234:  [#1] SMP PTI
[   74.381294] CPU: 3 PID: 144 Comm: xenwatch Not tainted 5.14.0-rc1+ #370
[   74.386172] Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
[   74.390918] RIP: 0010:del_timer+0x1f/0x80
[   74.394282] Code: 71 af a3 00 eb c1 31 c0 c3 66 90 0f 1f 44 00 00 41 55 41 
54 45 31 e4 55 48 83 ec 10 65 48 8b 04 25 28 00 00 00 48 89 44 24 08 <48> 8b 47 
08 48 85 c0 74 2d 48 89 e6 48 89 fd e8 dd e8 ff ff 48 89
[   74.407591] RSP: 0018:bab68423bcc8 EFLAGS: 00010082
[   74.411691] RAX: dd931e09aefb8f00 RBX: b0fa8ce8ee8a21dc RCX: 5e7f
[   74.417041] RDX: 5e80 RSI: 0001 RDI: b0fa8ce8ee8a222c
[   74.422425] RBP: bab68423bd20 R08: 0001 R09: 0001
[   74.427595] R10: 0001 R11: 0002 R12: 
[   74.432886] R13: a0484f3e4000 R14:  R15: a0484691c000
[   74.438784] FS:  () GS:a083c8e0() 
knlGS:
[   74.444592] CS:  0010 DS:  ES:  CR0: 80050033
[   74.448917] CR2: 7ff618903ff8 CR3: 000111e16001 CR4: 001706e0
[   74.454309] DR0:  DR1:  DR2: 
[   74.460128] DR3: 

Re: [PATCH v2] SUPPORT.md: add Dom0less as Supported

[Julien Grall]

Hi Stefano,

On 15/07/2021 00:48, Stefano Stabellini wrote:

Add Dom0less to SUPPORT.md to clarify its support status. The feature is
mature enough and small enough to make it security supported.


I would suggest to explain the restriction in the commit message (and 
give a link to XSA-372 commit).



Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- clarify memory scrubbing
---
  SUPPORT.md | 9 +
  1 file changed, 9 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index 317392d8f3..524cab9c8d 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -832,6 +832,15 @@ OVMF firmware implements the UEFI boot protocol.
  
  Status, qemu-xen: Supported
  
+## Dom0less

+
+Guest creation from the hypervisor at boot without Dom0 intervention.
+
+Status, ARM: Supported
+
+Memory of dom0less DomUs is not scrubbed at boot (even with
+bootscrub=on); no XSAs will be issues due to unscrubbed memory.


The memory will not be scrubbed for bootscrub=on and bootscrub=off. 
However, it should be scrubbed for bootscrub=idle (the default).


Cheers,

--
Julien Grall

Re: [PATCH v2 13/13] SUPPORT.md: write down restriction of 32-bit tool stacks

[Julien Grall]

Hi Jan,

On 15/07/2021 07:38, Jan Beulich wrote:

On 14.07.2021 20:16, Julien Grall wrote:

On 05/07/2021 16:18, Jan Beulich wrote:

Let's try to avoid giving the impression that 32-bit tool stacks are as
capable as 64-bit ones.


Would you be able to provide a few examples of the known issues in the
commit message? This would be helpful for anyone to understand why we
decided to drop the support.


Not sure how useful this is going to be.


It would at least be useful to me, so I can make an informed decision. I 
suspect it would also be for anyone reading it in the future. This is 
rather frustrating to find commit message with barely any rationale and 
no-one remembering why this was done...


I vaguely recall a discussion about 64-bit hypercall ([1]). I assume the 
decision to drop support is related to it, but I have no way to prove it 
from the commit message.


It is also not clear why adding the restriction is the way to go...


This would be pointing at the
declarations / definitions of various tool stack internal variables or
structure fields. Which also is why ...


... is this because such issues are too widespread in libxc/libxl to fix 
it in long term?





At least on Arm, we tried to design the hypercall ABI in such a way that
it should be possible to use a 32-bit toolstack.


... keeping the ABI tidy in this regard didn't help at all (albeit it
of course was a prereq to writing a tool stack that would be capable).

Jan



[1] 
https://lore.kernel.org/xen-devel/71b8a4f1-9c18-36e7-56b1-3f1b1dabd...@suse.com/


--
Julien Grall

Re: preparations for 4.15.1 and 4.13.4

[Anthony PERARD]

Can we backport support of QEMU 6.0 to Xen 4.15? I'm pretty sure
distributions are going to want to use the latest QEMU and latest Xen,
without needed to build two different QEMU binaries.

[XEN PATCH v2 0/8] Fix libxl with QEMU 6.0 + remove some more deprecated usages.
<20210511092810.13759-1-anthony.per...@citrix.com>
Commits: d5f54009db^..fe6630ddc4

Some more QEMU 6.0 fixes
<20210628100157.5010-1-anthony.per...@citrix.com>
Commits: 217eef30f7  3bc3be978f


Also, Olaf want them to be backported to 4.14, see
<20210629095952.7b0b94c1.o...@aepfle.de>

Thanks,

-- 
Anthony PERARD

Ping: [PATCH] xen-netback: correct success/error reporting for the SKB-with-fraglist case

[Jan Beulich]

On 20.05.2021 13:46, Jan Beulich wrote:
> On 25.02.2021 17:23, Paul Durrant wrote:
>> On 25/02/2021 14:00, Jan Beulich wrote:
>>> On 25.02.2021 13:11, Paul Durrant wrote:
 On 25/02/2021 07:33, Jan Beulich wrote:
> On 24.02.2021 17:39, Paul Durrant wrote:
>> On 23/02/2021 16:29, Jan Beulich wrote:
>>> When re-entering the main loop of xenvif_tx_check_gop() a 2nd time, the
>>> special considerations for the head of the SKB no longer apply. Don't
>>> mistakenly report ERROR to the frontend for the first entry in the list,
>>> even if - from all I can tell - this shouldn't matter much as the 
>>> overall
>>> transmit will need to be considered failed anyway.
>>>
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/drivers/net/xen-netback/netback.c
>>> +++ b/drivers/net/xen-netback/netback.c
>>> @@ -499,7 +499,7 @@ check_frags:
>>>  * the header's copy failed, and they 
>>> are
>>>  * sharing a slot, send an error
>>>  */
>>> -   if (i == 0 && sharedslot)
>>> +   if (i == 0 && !first_shinfo && 
>>> sharedslot)
>>> xenvif_idx_release(queue, 
>>> pending_idx,
>>>
>>> XEN_NETIF_RSP_ERROR);
>>> else
>>>
>>
>> I think this will DTRT, but to my mind it would make more sense to clear
>> 'sharedslot' before the 'goto check_frags' at the bottom of the function.
>
> That was my initial idea as well, but
> - I think it is for a reason that the variable is "const".
> - There is another use of it which would then instead need further
> amending (and which I believe is at least part of the reason for
> the variable to be "const").
>

 Oh, yes. But now that I look again, don't you want:

 if (i == 0 && first_shinfo && sharedslot)

 ? (i.e no '!')

 The comment states that the error should be indicated when the first
 frag contains the header in the case that the map succeeded but the
 prior copy from the same ref failed. This can only possibly be the case
 if this is the 'first_shinfo'
>>>
>>> I don't think so, no - there's a difference between "first frag"
>>> (at which point first_shinfo is NULL) and first frag list entry
>>> (at which point first_shinfo is non-NULL).
>>
>> Yes, I realise I got it backwards. Confusing name but the comment above 
>> its declaration does explain.
>>
>>>
 (which is why I still think it is safe to unconst 'sharedslot' and
 clear it).
>>>
>>> And "no" here as well - this piece of code
>>>
>>> /* First error: if the header haven't shared a slot with the
>>>  * first frag, release it as well.
>>>  */
>>> if (!sharedslot)
>>> xenvif_idx_release(queue,
>>>XENVIF_TX_CB(skb)->pending_idx,
>>>XEN_NETIF_RSP_OKAY);
>>>
>>> specifically requires sharedslot to have the value that was
>>> assigned to it at the start of the function (this property
>>> doesn't go away when switching from fragments to frag list).
>>> Note also how it uses XENVIF_TX_CB(skb)->pending_idx, i.e. the
>>> value the local variable pending_idx was set from at the start
>>> of the function.
>>>
>>
>> True, we do have to deal with freeing up the header if the first map 
>> error comes on the frag list.
>>
>> Reviewed-by: Paul Durrant 
> 
> Since I've not seen this go into 5.13-rc, may I ask what the disposition
> of this is?

I can't seem to spot this in 5.14-rc either. I have to admit I'm
increasingly puzzled ...

Jan

Re: [PATCH v2] SUPPORT.md: add Dom0less as Supported

[Bertrand Marquis]

Hi Stefano,


> On 15 Jul 2021, at 00:48, Stefano Stabellini  wrote:
> 
> Add Dom0less to SUPPORT.md to clarify its support status. The feature is
> mature enough and small enough to make it security supported.
> 
> Signed-off-by: Stefano Stabellini 
Reviewed-by: Bertrand Marquis 

Cheers
Bertrand

> ---
> Changes in v2:
> - clarify memory scrubbing
> ---
> SUPPORT.md | 9 +
> 1 file changed, 9 insertions(+)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index 317392d8f3..524cab9c8d 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -832,6 +832,15 @@ OVMF firmware implements the UEFI boot protocol.
> 
> Status, qemu-xen: Supported
> 
> +## Dom0less
> +
> +Guest creation from the hypervisor at boot without Dom0 intervention.
> +
> +Status, ARM: Supported
> +
> +Memory of dom0less DomUs is not scrubbed at boot (even with
> +bootscrub=on); no XSAs will be issues due to unscrubbed memory.
> +
> # Format and definitions
> 
> This file contains prose, and machine-readable fragments.
> -- 
> 2.17.1
> 
> 

[xen-unstable test] 163690: regressions - FAIL

[osstest service owner]

flight 163690 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/163690/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-amd 16 xen-boot/l1 fail REGR. vs. 163458

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit1   8 xen-boot fail in 163671 pass in 163690
 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeat  fail pass in 163671

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 163458
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 163458
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 163458
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 163458
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 163458
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 163458
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 163458
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 163458
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 163458
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 163458
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass

version targeted for testing:
 xen  3a98c1a4cec1a302beaddf944ded240b61173f87
baseline version:
 xen  0f435e2b58543f5baae96e17a10ae20d3dbc28fa

Last test of basis   163458  2021-07-08 23:09:08 Z6 days
Failing since163478  2021-07-09 15:23:45 Z5 days   11 attempts
Testing same since   163671  2021-07-14 05:26:47 Z1 days2 attempts

[ovmf test] 163691: regressions - FAIL

[osstest service owner]

flight 163691 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/163691/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 162359
 test-amd64-amd64-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 
162359

version targeted for testing:
 ovmf be282b14938846960cce30825a9fe762e14ca8c9
baseline version:
 ovmf c410ad4da4b7785170d3d42a3ba190c2caac6feb

Last test of basis   162359  2021-06-04 03:40:08 Z   41 days
Failing since162368  2021-06-04 15:42:59 Z   40 days  115 attempts
Testing same since   163676  2021-07-14 08:10:10 Z1 days2 attempts


People who touched revisions under test:
  Abner Chang 
  Agrawal, Sachin 
  Alexandru Elisei 
  Anthony PERARD 
  Ard Biesheuvel 
  Ashraf Ali S 
  Bob Feng 
  Bret Barkelew 
  Chen, Christine 
  Corvin Köhne 
  Daniel Schaefer 
  Daoxiang Li 
  Dov Murik 
  DunTan 
  gaoliming 
  Guo Dong 
  Hao A Wu 
  Jian J Wang 
  Jianyong Wu 
  Kaaira Gupta 
  Ken Lautner 
  Kenneth Lautner 
  Kun Qin 
  Laszlo Ersek 
  Leif Lindholm 
  Liming Gao 
  Liu, Zhiguang 
  Loo Tung Lun 
  Loo, Tung Lun 
  Manickavasakam Karpagavinayagam 
  Maurice Ma 
  Michael D Kinney 
  Neal Gompa 
  Ni, Ray 
  Nickle Wang 
  Patrick Rudolph 
  Pierre Gondois 
  Ray Ni 
  Rebecca Cran 
  Rebecca Cran 
  S, Ashraf Ali 
  Sachin Agrawal 
  Sami Mujawar 
  Scottie Kuo 
  Sean Brogan 
  Sean Brogan 
  Sheng Wei 
  Sumana Venur 
  Sunil V L 
  Tan, Dun 
  Thiyagu Kesavan Balakrishnan 
  Trammell Hudson 
  xueshengfeng 
  Yuwei Chen 
  Zhiguang Liu 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 3827 lines long.)

Re: preparations for 4.15.1 and 4.13.4

[Jan Beulich]

Andrew,

On 15.07.2021 09:58, Jan Beulich wrote:
> the releases are due in a couple of weeks time (and 4.14.3 is
> supposed to follow another few weeks later). Please point out backports
> you find missing from the respective staging branches, but which you
> consider relevant.
> 
> Please note that 4.13.4 is going to be the last Xen Project
> coordinated release from the 4.13 branch; the branch will go into
> security-only maintenance mode after this release.

as I don't suppose "x86/cpuid: Fix HLE and RTM handling (again)" is
what you were meaning to be all that's needed to fix my backport of
"x86/cpuid: Rework HLE and RTM handling" on the 4.13 branch, would
you please either submit whatever further fix you deem necessary,
or share enough information for someone else (perhaps me) to create
such a fix?

Thanks, Jan

preparations for 4.15.1 and 4.13.4

[Jan Beulich]

All,

the releases are due in a couple of weeks time (and 4.14.3 is
supposed to follow another few weeks later). Please point out backports
you find missing from the respective staging branches, but which you
consider relevant.

Please note that 4.13.4 is going to be the last Xen Project
coordinated release from the 4.13 branch; the branch will go into
security-only maintenance mode after this release.

Ian: I did take the liberty to backport Anthony's

5d3e4ebb5c71 libs/foreignmemory: Fix osdep_xenforeignmemory_map prototype

Beyond this I'd like the following to be considered:

6409210a5f51 libxencall: osdep_hypercall() should return long
bef64f2c0019 libxencall: introduce variant of xencall2() returning long
01a2d001dea2 libxencall: Bump SONAME following new functionality
6f02d1ea4a10 libxc: use multicall for memory-op on Linux (and Solaris)

If those are to be taken (which means in particular if the question of
the .so versioning can be properly sorted),

198a2bc6f149 x86/HVM: wire up multicalls

is going to be required as a prereq. I have backports of all of the
above ready (so I could put them in if you tell me to), but for
01a2d001dea2 only in its straightforward but simplistic form, which I'm
not sure is the right thing to do.

Jan

Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses

[Roman Skakun]

> This looks like it wasn't picked up? Should it go in rc1?

Hi, Konrad!

This looks like an unambiguous bug, and should be in rc1.

Cheers!

ср, 14 июл. 2021 г. в 03:15, Konrad Rzeszutek Wilk :
>
> On Tue, Jun 22, 2021 at 04:34:14PM +0300, Roman Skakun wrote:
> > This commit is dedicated to fix incorrect conversion from
> > cpu_addr to page address in cases when we get virtual
> > address which allocated in the vmalloc range.
> > As the result, virt_to_page() cannot convert this address
> > properly and return incorrect page address.
> >
> > Need to detect such cases and obtains the page address using
> > vmalloc_to_page() instead.
> >
> > Signed-off-by: Roman Skakun 
> > Reviewed-by: Andrii Anisov 
> > ---
> > Hey!
> > Thanks for suggestions, Christoph!
> > I updated the patch according to your advice.
> > But, I'm so surprised because nobody catches this problem
> > in the common code before. It looks a bit strange as for me.
>
> This looks like it wasn't picked up? Should it go in rc1?
> >
> >
> >  kernel/dma/ops_helpers.c | 12 ++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/dma/ops_helpers.c b/kernel/dma/ops_helpers.c
> > index 910ae69cae77..782728d8a393 100644
> > --- a/kernel/dma/ops_helpers.c
> > +++ b/kernel/dma/ops_helpers.c
> > @@ -5,6 +5,14 @@
> >   */
> >  #include 
> >
> > +static struct page *cpu_addr_to_page(void *cpu_addr)
> > +{
> > + if (is_vmalloc_addr(cpu_addr))
> > + return vmalloc_to_page(cpu_addr);
> > + else
> > + return virt_to_page(cpu_addr);
> > +}
> > +
> >  /*
> >   * Create scatter-list for the already allocated DMA buffer.
> >   */
> > @@ -12,7 +20,7 @@ int dma_common_get_sgtable(struct device *dev, struct 
> > sg_table *sgt,
> >void *cpu_addr, dma_addr_t dma_addr, size_t size,
> >unsigned long attrs)
> >  {
> > - struct page *page = virt_to_page(cpu_addr);
> > + struct page *page = cpu_addr_to_page(cpu_addr);
> >   int ret;
> >
> >   ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> > @@ -43,7 +51,7 @@ int dma_common_mmap(struct device *dev, struct 
> > vm_area_struct *vma,
> >   return -ENXIO;
> >
> >   return remap_pfn_range(vma, vma->vm_start,
> > - page_to_pfn(virt_to_page(cpu_addr)) + vma->vm_pgoff,
> > + page_to_pfn(cpu_addr_to_page(cpu_addr)) + 
> > vma->vm_pgoff,
> >   user_count << PAGE_SHIFT, vma->vm_page_prot);
> >  #else
> >   return -ENXIO;
> > --
> > 2.25.1
> >



-- 
Best Regards, Roman.

Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses

[Roman Skakun]

Hi, Stefano!

> We have the same thing in xen_swiotlb_free_coherent. Can we find a way
> to call cpu_addr_to_page() from xen_swiotlb_free_coherent?
> Maybe we can make cpu_addr_to_page non-static?

Yes, right, If we want to reuse this helper instead of the same code
block in xen_swiotlb_free_coherent() need to make cpu_addr_to_page() as
global and add a new declaration for this helper in include/linux/dma-map-ops.h.
What do you think?

Cheers!

ср, 14 июл. 2021 г. в 04:23, Stefano Stabellini :
>
> On Tue, 22 Jun 2021, Roman Skakun wrote:
> > This commit is dedicated to fix incorrect conversion from
> > cpu_addr to page address in cases when we get virtual
> > address which allocated in the vmalloc range.
> > As the result, virt_to_page() cannot convert this address
> > properly and return incorrect page address.
> >
> > Need to detect such cases and obtains the page address using
> > vmalloc_to_page() instead.
> >
> > Signed-off-by: Roman Skakun 
> > Reviewed-by: Andrii Anisov 
> > ---
> > Hey!
> > Thanks for suggestions, Christoph!
> > I updated the patch according to your advice.
> > But, I'm so surprised because nobody catches this problem
> > in the common code before. It looks a bit strange as for me.
> >
> >
> >  kernel/dma/ops_helpers.c | 12 ++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/dma/ops_helpers.c b/kernel/dma/ops_helpers.c
> > index 910ae69cae77..782728d8a393 100644
> > --- a/kernel/dma/ops_helpers.c
> > +++ b/kernel/dma/ops_helpers.c
> > @@ -5,6 +5,14 @@
> >   */
> >  #include 
> >
> > +static struct page *cpu_addr_to_page(void *cpu_addr)
> > +{
> > + if (is_vmalloc_addr(cpu_addr))
> > + return vmalloc_to_page(cpu_addr);
> > + else
> > + return virt_to_page(cpu_addr);
> > +}
>
> We have the same thing in xen_swiotlb_free_coherent. Can we find a way
> to call cpu_addr_to_page() from xen_swiotlb_free_coherent? Maybe we can
> make cpu_addr_to_page non-static? No problem if it is too much trouble.
>
>
> >  /*
> >   * Create scatter-list for the already allocated DMA buffer.
> >   */
> > @@ -12,7 +20,7 @@ int dma_common_get_sgtable(struct device *dev, struct 
> > sg_table *sgt,
> >void *cpu_addr, dma_addr_t dma_addr, size_t size,
> >unsigned long attrs)
> >  {
> > - struct page *page = virt_to_page(cpu_addr);
> > + struct page *page = cpu_addr_to_page(cpu_addr);
> >   int ret;
> >
> >   ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> > @@ -43,7 +51,7 @@ int dma_common_mmap(struct device *dev, struct 
> > vm_area_struct *vma,
> >   return -ENXIO;
> >
> >   return remap_pfn_range(vma, vma->vm_start,
> > - page_to_pfn(virt_to_page(cpu_addr)) + vma->vm_pgoff,
> > + page_to_pfn(cpu_addr_to_page(cpu_addr)) + 
> > vma->vm_pgoff,
> >   user_count << PAGE_SHIFT, vma->vm_page_prot);
> >  #else
> >   return -ENXIO;
> > --
> > 2.25.1
> >



-- 
Best Regards, Roman.

Re: [PATCH 1/4] build: use common stubs for debugger_trap_* functions if !CONFIG_CRASH_DEBUG

[Jan Beulich]

On 14.07.2021 23:03, Bob Eshleman wrote:
> On 7/14/21 2:34 AM, Jan Beulich wrote:
>>> +static inline bool debugger_trap_fatal(
>>> +unsigned int vector, const struct cpu_user_regs *regs)
>>
>> I'm afraid the concept of a vector may not be arch-independent.
>>
> 
> The only way I can imagine it not being arch-independent
> is if it is thought of as a trap number or id, instead of
> implying an entry in a vectored trap table.  I don't
> really understand this subsystem, so I'm probably missing
> context.
> 
> Are you suggesting a rename or a different approach entirely?

I'm suggesting that we shouldn't be claiming something to be an
abstraction when it isn't really. There's exactly one use of
debugger_trap_fatal() outside x86/ after patch 1 of this series:

static void do_debugger_trap_fatal(struct cpu_user_regs *regs)
{
(void)debugger_trap_fatal(0xf001, regs);
...
}

That's very certainly _not_ arch-independent. Hence I'd rather
see some #ifdef-ary added there and the function remaining
x86-specific for the time being, i.e. until such a time when
someone might come forward with a suitable abstraction. Perhaps
(as an alternative to #ifdef-ary) the '%' debug key should be
x86-specific altogether, and perhaps its setup and handling
could be moved into the new debugger.c?

Jan

Re: [PATCH v2 13/13] SUPPORT.md: write down restriction of 32-bit tool stacks

[Jan Beulich]

On 14.07.2021 20:16, Julien Grall wrote:
> On 05/07/2021 16:18, Jan Beulich wrote:
>> Let's try to avoid giving the impression that 32-bit tool stacks are as
>> capable as 64-bit ones.
> 
> Would you be able to provide a few examples of the known issues in the 
> commit message? This would be helpful for anyone to understand why we 
> decided to drop the support.

Not sure how useful this is going to be. This would be pointing at the
declarations / definitions of various tool stack internal variables or
structure fields. Which also is why ...

> At least on Arm, we tried to design the hypercall ABI in such a way that 
> it should be possible to use a 32-bit toolstack.

... keeping the ABI tidy in this regard didn't help at all (albeit it
of course was a prereq to writing a tool stack that would be capable).

Jan

Re: [PATCH v2 1/4] arm/traps: remove debugger_trap_fatal() calls

[Jan Beulich]

On 14.07.2021 22:37, Bobby Eshleman wrote:
> ARM doesn't actually use debugger_trap_* anything, and is stubbed out.
> 
> Simply remove the calls. This also renders TRAP_invalid_op unused in
> any common code, so remove that definition too.

This part of the description is now stale; I guess if no other need
arises to resubmit the committer could take care of removing this.

Jan

Re: [XEN PATCH] xen: allow XSM_FLASK_POLICY only if checkpolicy binary is available

[Jan Beulich]

On 14.07.2021 18:17, Anthony PERARD wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -25,6 +25,9 @@ config GRANT_TABLE
>  config HAS_ALTERNATIVE
>   bool
>  
> +config HAS_CHECKPOLICY
> + def_bool $(success,$(CHECKPOLICY) -h 2>&1 | grep -q xen)
> +

This is no different from other aspects of "Kconfig vs tool chain
capabilities" sent out last August to start a discussion about
whether we really want such. Besides Jürgen no-one cared to reply
iirc, which to me means no-one really cares one way or the other.
Which I didn't think was the case ... So here we are again, with
all the same questions still open.

I'm not going to nack the patch, because there's an immediate
purpose / need, but I also can't avoid commenting (and I won't
put my name on it in any positive way, i.e. also not as a
committer; if anything then to record my reservations).

Independent of this I'd like to raise the question of whether
the chosen placement is optimal. Other capability checks live
in xen/Kconfig.

Jan