> [Severity: High] > Does this ignore the immediate need to restore mappings for the current > reverted endpoint?
That path is the ATTACH request *failing*, and there the endpoint cannot have moved: the request could not be queued, so nothing reached the device. I checked this with a failslab injection on the ATTACH request: the trace shows no ATTACH for the target domain at all, and the endpoint stays on the domain it was on - which, in the flows here, is the core's empty blocking domain. In the other failure mode - a device rejecting an ATTACH it has received - the driver still assumes the endpoint stayed where it was; that is the same assumption the baseline made. With QEMU, the only rejection that happens after it detached the endpoint from its old domain needs a bypass flag that does not match the target domain, and this driver cannot send one: it only sets BYPASS for the domain it reserved at probe time. So the old domain still holds the endpoint and all of its mappings, and there is nothing to restore here. replay_pending is only re-armed conservatively, for the case where the old domain's count had reached zero while the endpoint was retired. > [Severity: High] > If viommu_replay_mappings() fails during viommu_attach_dev(), the endpoint > is reverted to its old domain on the hardware. > Since the old domain's mappings were dropped by the device when the > endpoint initially moved, does the reverted domain now lack all mappings > on the physical device? > Could this cause all subsequent DMA from this device to fault silently, as > the OS assumes the old domain is still fully intact? This one is deliberate, and it is bounded. After a failed replay the endpoint is put back on the old domain, whose device-side mappings the device dropped when the endpoint left it; they are re-sent by the next attach to that domain, which replay_pending forces - the flag is set exactly when the old domain's count reached zero, i.e. when the device dropped it. What bounds the window until then: in the flows this series targets, the old domain is the empty blocking/identity domain that the core attaches before the user's domain, so there is nothing that can be missing. If the old domain is a populated one, the state is still better than the baseline's: the baseline left the device on the *new* domain (with its partial mappings) while the driver and the core believed the endpoint was on the old one - the same missing mappings on the wrong domain, plus a count that the next attach would underflow. If you would rather have the old domain whole again before the failing attach returns, that is one extra call: replay old_vdomain right after a successful revert (a failure there leaves replay_pending set, as now). I left it out because it adds a full replay - potentially as expensive as the one that just failed - to an error path that the next attach to that domain repairs anyway. Happy to add it if that is preferred. The comments in the v2 I just sent say this in the code, so a reader does not need this thread: the ATTACH-failure exit states that the endpoint is still on the old domain with its mappings and that nothing has to be replayed, and the failed-replay exit states that the old domain's replay is left to its next attach. Thanks, Anlai
