On Thu, 20 Apr 2023 15:19:55 +0100 Robin Murphy <robin.murphy@xxxxxxx> wrote: > On 2023-04-20 15:15, Alex Williamson wrote: > > On Thu, 20 Apr 2023 06:52:01 +0000 > > "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote: > > > >> Hi, Alex, > >> > >> Happen to see that we may have inconsistent policy about RMRR devices cross > >> different vendors. > >> > >> Previously only Intel supports RMRR. Now both AMD/ARM have similar thing, > >> AMD IVMD and ARM RMR. > > > > Any similar requirement imposed by system firmware that the operating > > system must perpetually maintain a specific IOVA mapping for the device > > should impose similar restrictions as we've implemented for VT-d > > RMMR[1]. Thanks, > > Hmm, does that mean that vfio_iommu_resv_exclude() going to the trouble > of punching out all the reserved region holes isn't really needed? While "Reserved Memory Region Reporting", might suggest that the ranges are simply excluded, RMRR actually require that specific mappings are maintained for ongoing, side-band activity, which is not compatible with the ideas that userspace owns the IOVA address space for the device or separation of host vs userspace control of the device. Such mappings suggest things like system health monitoring where the influence of a user-owned device can easily extend to a system-wide scope if the user it able to manipulate the device to deny that interaction or report bad data. If these ARM and AMD tables impose similar requirements, we should really be restricting devices encumbered by such requirements from userspace access as well. Thanks, Alex > > [1]https://access.redhat.com/sites/default/files/attachments/rmrr-wp1.pdf > > > >> RMRR identity mapping was considered unsafe (except for USB/GPU) for > >> device assignment: > >> > >> /* > >> * There are a couple cases where we need to restrict the functionality of > >> * devices associated with RMRRs. The first is when evaluating a device for > >> * identity mapping because problems exist when devices are moved in and out > >> * of domains and their respective RMRR information is lost. This means that > >> * a device with associated RMRRs will never be in a "passthrough" domain. > >> * The second is use of the device through the IOMMU API. This interface > >> * expects to have full control of the IOVA space for the device. We cannot > >> * satisfy both the requirement that RMRR access is maintained and have an > >> * unencumbered IOVA space. We also have no ability to quiesce the device's > >> * use of the RMRR space or even inform the IOMMU API user of the restriction. > >> * We therefore prevent devices associated with an RMRR from participating in > >> * the IOMMU API, which eliminates them from device assignment. > >> * > >> * In both cases, devices which have relaxable RMRRs are not concerned by this > >> * restriction. See device_rmrr_is_relaxable comment. > >> */ > >> static bool device_is_rmrr_locked(struct device *dev) > >> { > >> if (!device_has_rmrr(dev)) > >> return false; > >> > >> if (device_rmrr_is_relaxable(dev)) > >> return false; > >> > >> return true; > >> } > >> > >> Then non-relaxable RMRR device is rejected when doing attach: > >> > >> static int intel_iommu_attach_device(struct iommu_domain *domain, > >> struct device *dev) > >> { > >> struct device_domain_info *info = dev_iommu_priv_get(dev); > >> int ret; > >> > >> if (domain->type == IOMMU_DOMAIN_UNMANAGED && > >> device_is_rmrr_locked(dev)) { > >> dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement. Contact your platform vendor.\n"); > >> return -EPERM; > >> } > >> ... > >> } > >> > >> But I didn't find the same check in AMD/ARM driver at a glance. > >> > >> Did I overlook some arch difference which makes RMRR device safe in > >> those platforms or is it a gap to be fixed? > >> > >> Thanks > >> Kevin > >> > > >