On 2/4/25 03:11, Jason Gunthorpe wrote:
On Tue, Feb 18, 2025 at 10:10:00PM +1100, Alexey Kardashevskiy wrote:
@@ -939,6 +939,7 @@ struct iommu_fault_alloc {
enum iommu_viommu_type {
IOMMU_VIOMMU_TYPE_DEFAULT = 0,
IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1,
+ IOMMU_VIOMMU_TYPE_TSM = 2,
This should probably be some kind of AMD_TSM and the driver data blob
should carry any additional data needed to create the vIOMMU that is
visible to the guest.
@@ -2068,7 +2069,18 @@ static void set_dte_entry(struct amd_iommu *iommu,
new.data[1] |= DTE_FLAG_IOTLB;
old_domid = READ_ONCE(dte->data[1]) & DEV_DOMID_MASK;
- new.data[1] |= domid;
+
+ if (domain->aviommu) {
AMD should be implementing viommu natively without CC as well, try to
structure things so it fits together better. This should only trigger
for the CC viommu type..
+ /*
+ * This runs when VFIO is bound to a device but TDI is not yet.
+ * Ideally TSM should change DTE only when TDI is bound.
+ */
+ dev_info(dev_data->dev, "Skip DomainID=%x and set bit96\n", domid);
+ new.data[1] |= 1ULL << (96 - 64);
+ } else {
+ dev_info(dev_data->dev, "Not skip DomainID=%x and not set bit96\n", domid);
+ new.data[1] |= domid;
+ }
/*
* Restore cached persistent DTE bits, which can be set by information
@@ -2549,12 +2561,15 @@ amd_iommu_domain_alloc_paging_flags(struct device *dev, u32 flags,
{
struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
const u32 supported_flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
+ IOMMU_HWPT_ALLOC_PASID |
+ IOMMU_HWPT_ALLOC_NEST_PARENT;
+ const u32 supported_flags2 = IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
IOMMU_HWPT_ALLOC_PASID;
Just ignore NEST_PARENT? That seems wrong, it should force a V1 page
table??
Ahhh... This is because I still have troubles with what
IOMMU_DOMAIN_NESTED means (and iommufd.rst does not help me). There is
one device, one IOMMU table buuut 2 domains? Uh.
+static struct iommufd_viommu *amd_viommu_alloc(struct device *dev,
+ struct iommu_domain *parent,
+ struct iommufd_ctx *ictx,
+ unsigned int viommu_type)
+{
+ struct amd_viommu *aviommu;
+ struct protection_domain *domain = to_pdomain(parent);
+
+ if (viommu_type != IOMMU_VIOMMU_TYPE_TSM)
+ return ERR_PTR(-EOPNOTSUPP);
+
+ aviommu = iommufd_viommu_alloc(ictx, struct amd_viommu, core, &amd_viommu_ops);
+ if (IS_ERR(aviommu))
+ return ERR_CAST(aviommu);
+
+ aviommu->domain = domain;
This is not OK, the parent domain of the viommu can be used with
multiple viommu objects, it can't just have a naked back reference
like this.
You can get 1:1 domain objects linked to the viommu by creating the
'S1' type domains, maybe that is what you want here. A special domain
type that is TSM that has a special DTE.
Should not IOMMU_DOMAIN_NESTED be that "S1" domain? And what does "S1"
mean here? Currently the domain in the hunk above is __IOMMU_DOMAIN_PAGING.
Though I'd really rather see the domain attach logic and DTE formation
in the AMD driver be fixed up before we made it more complex :\
It would be nice to see normal nesting and viommu support first too :\
It is in the works too. Thanks,
--
Alexey
