On 20/04/2023 3:49 pm, Alex Williamson wrote:
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,
Indeed the primary use-case behind Arm's RMRs was certain devices like
big complex RAID controllers which have already been started by UEFI
firmware at boot and have live in-memory data which needs to be preserved.
However, my point was more that if it's a VFIO policy that any device
with an IOMMU_RESV_DIRECT reservation is not suitable for userspace
assignment, then vfio_iommu_type1_attach_group() already has everything
it would need to robustly enforce that policy itself. It seems silly to
me for it to expect the IOMMU driver to fail the attach, then go ahead
and dutifully punch out direct regions if it happened not to. A couple
of obvious trivial tweaks and there could be no dependency on driver
behaviour at all, other than correctly reporting resv_regions to begin with.
If we think this policy deserves to go beyond VFIO and userspace, and
it's reasonable that such devices should never be allowed to attach to
any other kind of kernel-owned unmanaged domain either, then we can
still trivially enforce that in core IOMMU code. I really see no need
for it to be in drivers at all.
Thanks,
Robin.
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
