Commit a13f0655 authored by Linus Torvalds's avatar Linus Torvalds

Merge tag 'iommu-updates-v5.2' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/joro/iommu

Pull IOMMU updates from Joerg Roedel:

 - ATS support for ARM-SMMU-v3.

 - AUX domain support in the IOMMU-API and the Intel VT-d driver. This
   adds support for multiple DMA address spaces per (PCI-)device. The
   use-case is to multiplex devices between host and KVM guests in a
   more flexible way than supported by SR-IOV.

 - the rest are smaller cleanups and fixes, two of which needed to be
   reverted after testing in linux-next.

* tag 'iommu-updates-v5.2' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/joro/iommu: (45 commits)
  Revert "iommu/amd: Flush not present cache in iommu_map_page"
  Revert "iommu/amd: Remove the leftover of bypass support"
  iommu/vt-d: Fix leak in intel_pasid_alloc_table on error path
  iommu/vt-d: Make kernel parameter igfx_off work with vIOMMU
  iommu/vt-d: Set intel_iommu_gfx_mapped correctly
  iommu/amd: Flush not present cache in iommu_map_page
  iommu/vt-d: Cleanup: no spaces at the start of a line
  iommu/vt-d: Don't request page request irq under dmar_global_lock
  iommu/vt-d: Use struct_size() helper
  iommu/mediatek: Fix leaked of_node references
  iommu/amd: Remove amd_iommu_pd_list
  iommu/arm-smmu: Log CBFRSYNRA register on context fault
  iommu/arm-smmu-v3: Don't disable SMMU in kdump kernel
  iommu/arm-smmu-v3: Disable tagged pointers
  iommu/arm-smmu-v3: Add support for PCI ATS
  iommu/arm-smmu-v3: Link domains and devices
  iommu/arm-smmu-v3: Add a master->domain pointer
  iommu/arm-smmu-v3: Store SteamIDs in master
  iommu/arm-smmu-v3: Rename arm_smmu_master_data to arm_smmu_master
  ACPI/IORT: Check ATS capability in root complex nodes
  ...
parents 55472bae b5531563
......@@ -1031,6 +1031,14 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
dev_dbg(dev, "dma_pfn_offset(%#08llx)\n", offset);
}
static bool iort_pci_rc_supports_ats(struct acpi_iort_node *node)
{
struct acpi_iort_root_complex *pci_rc;
pci_rc = (struct acpi_iort_root_complex *)node->node_data;
return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED;
}
/**
* iort_iommu_configure - Set-up IOMMU configuration for a device.
*
......@@ -1066,6 +1074,9 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
info.node = node;
err = pci_for_each_dma_alias(to_pci_dev(dev),
iort_pci_iommu_init, &info);
if (!err && iort_pci_rc_supports_ats(node))
dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
} else {
int i = 0;
......
......@@ -359,6 +359,31 @@ config ARM_SMMU
Say Y here if your SoC includes an IOMMU device implementing
the ARM SMMU architecture.
config ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT
bool "Default to disabling bypass on ARM SMMU v1 and v2"
depends on ARM_SMMU
default y
help
Say Y here to (by default) disable bypass streams such that
incoming transactions from devices that are not attached to
an iommu domain will report an abort back to the device and
will not be allowed to pass through the SMMU.
Any old kernels that existed before this KConfig was
introduced would default to _allowing_ bypass (AKA the
equivalent of NO for this config). However the default for
this option is YES because the old behavior is insecure.
There are few reasons to allow unmatched stream bypass, and
even fewer good ones. If saying YES here breaks your board
you should work on fixing your board. This KConfig option
is expected to be removed in the future and we'll simply
hardcode the bypass disable in the code.
NOTE: the kernel command line parameter
'arm-smmu.disable_bypass' will continue to override this
config.
config ARM_SMMU_V3
bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
depends on ARM64
......
......@@ -1723,31 +1723,6 @@ static void dma_ops_free_iova(struct dma_ops_domain *dma_dom,
*
****************************************************************************/
/*
* This function adds a protection domain to the global protection domain list
*/
static void add_domain_to_list(struct protection_domain *domain)
{
unsigned long flags;
spin_lock_irqsave(&amd_iommu_pd_lock, flags);
list_add(&domain->list, &amd_iommu_pd_list);
spin_unlock_irqrestore(&amd_iommu_pd_lock, flags);
}
/*
* This function removes a protection domain to the global
* protection domain list
*/
static void del_domain_from_list(struct protection_domain *domain)
{
unsigned long flags;
spin_lock_irqsave(&amd_iommu_pd_lock, flags);
list_del(&domain->list);
spin_unlock_irqrestore(&amd_iommu_pd_lock, flags);
}
static u16 domain_id_alloc(void)
{
int id;
......@@ -1838,8 +1813,6 @@ static void dma_ops_domain_free(struct dma_ops_domain *dom)
if (!dom)
return;
del_domain_from_list(&dom->domain);
put_iova_domain(&dom->iovad);
free_pagetable(&dom->domain);
......@@ -1880,8 +1853,6 @@ static struct dma_ops_domain *dma_ops_domain_alloc(void)
/* Initialize reserved ranges */
copy_reserved_iova(&reserved_iova_ranges, &dma_dom->iovad);
add_domain_to_list(&dma_dom->domain);
return dma_dom;
free_dma_dom:
......@@ -2122,23 +2093,6 @@ static int pdev_iommuv2_enable(struct pci_dev *pdev)
return ret;
}
/* FIXME: Move this to PCI code */
#define PCI_PRI_TLP_OFF (1 << 15)
static bool pci_pri_tlp_required(struct pci_dev *pdev)
{
u16 status;
int pos;
pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
if (!pos)
return false;
pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status);
return (status & PCI_PRI_TLP_OFF) ? true : false;
}
/*
* If a device is not yet associated with a domain, this function makes the
* device visible in the domain
......@@ -2167,7 +2121,7 @@ static int attach_device(struct device *dev,
dev_data->ats.enabled = true;
dev_data->ats.qdep = pci_ats_queue_depth(pdev);
dev_data->pri_tlp = pci_pri_tlp_required(pdev);
dev_data->pri_tlp = pci_prg_resp_pasid_required(pdev);
}
} else if (amd_iommu_iotlb_sup &&
pci_enable_ats(pdev, PAGE_SHIFT) == 0) {
......@@ -2897,8 +2851,6 @@ static void protection_domain_free(struct protection_domain *domain)
if (!domain)
return;
del_domain_from_list(domain);
if (domain->id)
domain_id_free(domain->id);
......@@ -2928,8 +2880,6 @@ static struct protection_domain *protection_domain_alloc(void)
if (protection_domain_init(domain))
goto out_err;
add_domain_to_list(domain);
return domain;
out_err:
......
......@@ -188,12 +188,6 @@ static bool amd_iommu_pc_present __read_mostly;
bool amd_iommu_force_isolation __read_mostly;
/*
* List of protection domains - used during resume
*/
LIST_HEAD(amd_iommu_pd_list);
spinlock_t amd_iommu_pd_lock;
/*
* Pointer to the device table which is shared by all AMD IOMMUs
* it is indexed by the PCI device id or the HT unit id and contains
......@@ -2526,8 +2520,6 @@ static int __init early_amd_iommu_init(void)
*/
__set_bit(0, amd_iommu_pd_alloc_bitmap);
spin_lock_init(&amd_iommu_pd_lock);
/*
* now the data structures are allocated and basically initialized
* start the real acpi table scan
......
......@@ -674,12 +674,6 @@ extern struct list_head amd_iommu_list;
*/
extern struct amd_iommu *amd_iommus[MAX_IOMMUS];
/*
* Declarations for the global list of all protection domains
*/
extern spinlock_t amd_iommu_pd_lock;
extern struct list_head amd_iommu_pd_list;
/*
* Structure defining one entry in the device table
*/
......
......@@ -147,6 +147,8 @@ enum arm_smmu_s2cr_privcfg {
#define CBAR_IRPTNDX_SHIFT 24
#define CBAR_IRPTNDX_MASK 0xff
#define ARM_SMMU_GR1_CBFRSYNRA(n) (0x400 + ((n) << 2))
#define ARM_SMMU_GR1_CBA2R(n) (0x800 + ((n) << 2))
#define CBA2R_RW64_32BIT (0 << 0)
#define CBA2R_RW64_64BIT (1 << 0)
......
This diff is collapsed.
......@@ -110,7 +110,8 @@ static int force_stage;
module_param(force_stage, int, S_IRUGO);
MODULE_PARM_DESC(force_stage,
"Force SMMU mappings to be installed at a particular stage of translation. A value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no stage is forced). Note that selecting a specific stage will disable support for nested translation.");
static bool disable_bypass;
static bool disable_bypass =
IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT);
module_param(disable_bypass, bool, S_IRUGO);
MODULE_PARM_DESC(disable_bypass,
"Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU.");
......@@ -569,12 +570,13 @@ static const struct iommu_gather_ops arm_smmu_s2_tlb_ops_v1 = {
static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
{
u32 fsr, fsynr;
u32 fsr, fsynr, cbfrsynra;
unsigned long iova;
struct iommu_domain *domain = dev;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
struct arm_smmu_device *smmu = smmu_domain->smmu;
void __iomem *gr1_base = ARM_SMMU_GR1(smmu);
void __iomem *cb_base;
cb_base = ARM_SMMU_CB(smmu, cfg->cbndx);
......@@ -585,10 +587,11 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
fsynr = readl_relaxed(cb_base + ARM_SMMU_CB_FSYNR0);
iova = readq_relaxed(cb_base + ARM_SMMU_CB_FAR);
cbfrsynra = readl_relaxed(gr1_base + ARM_SMMU_GR1_CBFRSYNRA(cfg->cbndx));
dev_err_ratelimited(smmu->dev,
"Unhandled context fault: fsr=0x%x, iova=0x%08lx, fsynr=0x%x, cb=%d\n",
fsr, iova, fsynr, cfg->cbndx);
"Unhandled context fault: fsr=0x%x, iova=0x%08lx, fsynr=0x%x, cbfrsynra=0x%x, cb=%d\n",
fsr, iova, fsynr, cbfrsynra, cfg->cbndx);
writel(fsr, cb_base + ARM_SMMU_CB_FSR);
return IRQ_HANDLED;
......
......@@ -145,7 +145,7 @@ dmar_alloc_pci_notify_info(struct pci_dev *dev, unsigned long event)
for (tmp = dev; tmp; tmp = tmp->bus->self)
level++;
size = sizeof(*info) + level * sizeof(info->path[0]);
size = struct_size(info, path, level);
if (size <= sizeof(dmar_pci_notify_info_buf)) {
info = (struct dmar_pci_notify_info *)dmar_pci_notify_info_buf;
} else {
......
This diff is collapsed.
......@@ -154,8 +154,10 @@ int intel_pasid_alloc_table(struct device *dev)
order = size ? get_order(size) : 0;
pages = alloc_pages_node(info->iommu->node,
GFP_KERNEL | __GFP_ZERO, order);
if (!pages)
if (!pages) {
kfree(pasid_table);
return -ENOMEM;
}
pasid_table->table = page_address(pages);
pasid_table->order = order;
......
......@@ -228,6 +228,7 @@ static LIST_HEAD(global_svm_list);
int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
{
struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
struct device_domain_info *info;
struct intel_svm_dev *sdev;
struct intel_svm *svm = NULL;
struct mm_struct *mm = NULL;
......@@ -291,13 +292,29 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
}
sdev->dev = dev;
ret = intel_iommu_enable_pasid(iommu, sdev);
ret = intel_iommu_enable_pasid(iommu, dev);
if (ret || !pasid) {
/* If they don't actually want to assign a PASID, this is
* just an enabling check/preparation. */
kfree(sdev);
goto out;
}
info = dev->archdata.iommu;
if (!info || !info->pasid_supported) {
kfree(sdev);
goto out;
}
sdev->did = FLPT_DEFAULT_DID;
sdev->sid = PCI_DEVID(info->bus, info->devfn);
if (info->ats_enabled) {
sdev->dev_iotlb = 1;
sdev->qdep = info->ats_qdep;
if (sdev->qdep >= QI_DEV_EIOTLB_MAX_INVS)
sdev->qdep = 0;
}
/* Finish the setup now we know we're keeping it */
sdev->users = 1;
sdev->ops = ops;
......
......@@ -548,8 +548,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu)
goto out_free_table;
}
bitmap = kcalloc(BITS_TO_LONGS(INTR_REMAP_TABLE_ENTRIES),
sizeof(long), GFP_ATOMIC);
bitmap = bitmap_zalloc(INTR_REMAP_TABLE_ENTRIES, GFP_ATOMIC);
if (bitmap == NULL) {
pr_err("IR%d: failed to allocate bitmap\n", iommu->seq_id);
goto out_free_pages;
......@@ -616,7 +615,7 @@ static int intel_setup_irq_remapping(struct intel_iommu *iommu)
return 0;
out_free_bitmap:
kfree(bitmap);
bitmap_free(bitmap);
out_free_pages:
__free_pages(pages, INTR_REMAP_PAGE_ORDER);
out_free_table:
......@@ -640,7 +639,7 @@ static void intel_teardown_irq_remapping(struct intel_iommu *iommu)
}
free_pages((unsigned long)iommu->ir_table->base,
INTR_REMAP_PAGE_ORDER);
kfree(iommu->ir_table->bitmap);
bitmap_free(iommu->ir_table->bitmap);
kfree(iommu->ir_table);
iommu->ir_table = NULL;
}
......
......@@ -45,10 +45,6 @@ static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
#endif
static bool iommu_dma_strict __read_mostly = true;
struct iommu_callback_data {
const struct iommu_ops *ops;
};
struct iommu_group {
struct kobject kobj;
struct kobject *devices_kobj;
......@@ -1217,9 +1213,6 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
{
int err;
struct notifier_block *nb;
struct iommu_callback_data cb = {
.ops = ops,
};
nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
if (!nb)
......@@ -1231,7 +1224,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
if (err)
goto out_free;
err = bus_for_each_dev(bus, NULL, &cb, add_iommu_group);
err = bus_for_each_dev(bus, NULL, NULL, add_iommu_group);
if (err)
goto out_err;
......@@ -1240,7 +1233,7 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops)
out_err:
/* Clean up */
bus_for_each_dev(bus, NULL, &cb, remove_iommu_group);
bus_for_each_dev(bus, NULL, NULL, remove_iommu_group);
bus_unregister_notifier(bus, nb);
out_free:
......@@ -2039,3 +2032,203 @@ int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
return 0;
}
EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
/*
* Per device IOMMU features.
*/
bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features feat)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
if (ops && ops->dev_has_feat)
return ops->dev_has_feat(dev, feat);
return false;
}
EXPORT_SYMBOL_GPL(iommu_dev_has_feature);
int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
if (ops && ops->dev_enable_feat)
return ops->dev_enable_feat(dev, feat);
return -ENODEV;
}
EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
/*
* The device drivers should do the necessary cleanups before calling this.
* For example, before disabling the aux-domain feature, the device driver
* should detach all aux-domains. Otherwise, this will return -EBUSY.
*/
int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
if (ops && ops->dev_disable_feat)
return ops->dev_disable_feat(dev, feat);
return -EBUSY;
}
EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features feat)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
if (ops && ops->dev_feat_enabled)
return ops->dev_feat_enabled(dev, feat);
return false;
}
EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
/*
* Aux-domain specific attach/detach.
*
* Only works if iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX) returns
* true. Also, as long as domains are attached to a device through this
* interface, any tries to call iommu_attach_device() should fail
* (iommu_detach_device() can't fail, so we fail when trying to re-attach).
* This should make us safe against a device being attached to a guest as a
* whole while there are still pasid users on it (aux and sva).
*/
int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
{
int ret = -ENODEV;
if (domain->ops->aux_attach_dev)
ret = domain->ops->aux_attach_dev(domain, dev);
if (!ret)
trace_attach_device_to_domain(dev);
return ret;
}
EXPORT_SYMBOL_GPL(iommu_aux_attach_device);
void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
{
if (domain->ops->aux_detach_dev) {
domain->ops->aux_detach_dev(domain, dev);
trace_detach_device_from_domain(dev);
}
}
EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
{
int ret = -ENODEV;
if (domain->ops->aux_get_pasid)
ret = domain->ops->aux_get_pasid(domain, dev);
return ret;
}
EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
/**
* iommu_sva_bind_device() - Bind a process address space to a device
* @dev: the device
* @mm: the mm to bind, caller must hold a reference to it
*
* Create a bond between device and address space, allowing the device to access
* the mm using the returned PASID. If a bond already exists between @device and
* @mm, it is returned and an additional reference is taken. Caller must call
* iommu_sva_unbind_device() to release each reference.
*
* iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called first, to
* initialize the required SVA features.
*
* On error, returns an ERR_PTR value.
*/
struct iommu_sva *
iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
{
struct iommu_group *group;
struct iommu_sva *handle = ERR_PTR(-EINVAL);
const struct iommu_ops *ops = dev->bus->iommu_ops;
if (!ops || !ops->sva_bind)
return ERR_PTR(-ENODEV);
group = iommu_group_get(dev);
if (!group)
return ERR_PTR(-ENODEV);
/* Ensure device count and domain don't change while we're binding */
mutex_lock(&group->mutex);
/*
* To keep things simple, SVA currently doesn't support IOMMU groups
* with more than one device. Existing SVA-capable systems are not
* affected by the problems that required IOMMU groups (lack of ACS
* isolation, device ID aliasing and other hardware issues).
*/
if (iommu_group_device_count(group) != 1)
goto out_unlock;
handle = ops->sva_bind(dev, mm, drvdata);
out_unlock:
mutex_unlock(&group->mutex);
iommu_group_put(group);
return handle;
}
EXPORT_SYMBOL_GPL(iommu_sva_bind_device);
/**
* iommu_sva_unbind_device() - Remove a bond created with iommu_sva_bind_device
* @handle: the handle returned by iommu_sva_bind_device()
*
* Put reference to a bond between device and address space. The device should
* not be issuing any more transaction for this PASID. All outstanding page
* requests for this PASID must have been flushed to the IOMMU.
*
* Returns 0 on success, or an error value
*/
void iommu_sva_unbind_device(struct iommu_sva *handle)
{
struct iommu_group *group;
struct device *dev = handle->dev;
const struct iommu_ops *ops = dev->bus->iommu_ops;
if (!ops || !ops->sva_unbind)
return;
group = iommu_group_get(dev);
if (!group)
return;
mutex_lock(&group->mutex);
ops->sva_unbind(handle);
mutex_unlock(&group->mutex);
iommu_group_put(group);
}
EXPORT_SYMBOL_GPL(iommu_sva_unbind_device);
int iommu_sva_set_ops(struct iommu_sva *handle,
const struct iommu_sva_ops *sva_ops)
{
if (handle->ops && handle->ops != sva_ops)
return -EEXIST;
handle->ops = sva_ops;
return 0;
}
EXPORT_SYMBOL_GPL(iommu_sva_set_ops);
int iommu_sva_get_pasid(struct iommu_sva *handle)
{
const struct iommu_ops *ops = handle->dev->bus->iommu_ops;
if (!ops || !ops->sva_get_pasid)
return IOMMU_PASID_INVALID;
return ops->sva_get_pasid(handle);
}
EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
......@@ -632,16 +632,20 @@ static int mtk_iommu_probe(struct platform_device *pdev)
if (!larbnode)
return -EINVAL;
if (!of_device_is_available(larbnode))
if (!of_device_is_available(larbnode)) {
of_node_put(larbnode);
continue;
}
ret = of_property_read_u32(larbnode, "mediatek,larb-id", &id);
if (ret)/* The id is consecutive if there is no this property */
id = i;
plarbdev = of_find_device_by_node(larbnode);
if (!plarbdev)
if (!plarbdev) {
of_node_put(larbnode);
return -EPROBE_DEFER;
}
data->smi_imu.larb_imu[id].dev = &plarbdev->dev;
component_match_add_release(dev, &match, release_of,
......
......@@ -102,7 +102,6 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu, unsigned long offset)
#define SMMU_TLB_FLUSH_VA_MATCH_ALL (0 << 0)
#define SMMU_TLB_FLUSH_VA_MATCH_SECTION (2 << 0)
#define SMMU_TLB_FLUSH_VA_MATCH_GROUP (3 << 0)
#define SMMU_TLB_FLUSH_ASID(x) (((x) & 0x7f) << 24)
#define SMMU_TLB_FLUSH_VA_SECTION(addr) ((((addr) & 0xffc00000) >> 12) | \
SMMU_TLB_FLUSH_VA_MATCH_SECTION)
#define SMMU_TLB_FLUSH_VA_GROUP(addr) ((((addr) & 0xffffc000) >> 12) | \
......@@ -146,8 +145,6 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu, unsigned long offset)
#define SMMU_PDE_ATTR (SMMU_PDE_READABLE | SMMU_PDE_WRITABLE | \
SMMU_PDE_NONSECURE)
#define SMMU_PTE_ATTR (SMMU_PTE_READABLE | SMMU_PTE_WRITABLE | \
SMMU_PTE_NONSECURE)
static unsigned int iova_pd_index(unsigned long iova)
{
......@@ -205,8 +202,12 @@ static inline void smmu_flush_tlb_asid(struct tegra_smmu *smmu,
{
u32 value;
value = SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_ASID(asid) |
SMMU_TLB_FLUSH_VA_MATCH_ALL;
if (smmu->soc->num_asids == 4)
value = (asid & 0x3) << 29;
else
value = (asid & 0x7f) << 24;
value |= SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_VA_MATCH_ALL;
smmu_writel(smmu, value, SMMU_TLB_FLUSH);
}
......@@ -216,8 +217,12 @@ static inline void smmu_flush_tlb_section(struct tegra_smmu *smmu,
{
u32 value;
value = SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_ASID(asid) |
SMMU_TLB_FLUSH_VA_SECTION(iova);
if (smmu->soc->num_asids == 4)
value = (asid & 0x3) << 29;
else
value = (asid & 0x7f) << 24;
value |= SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_VA_SECTION(iova);
smmu_writel(smmu, value, SMMU_TLB_FLUSH);
}
......@@ -227,8 +232,12 @@ static inline void smmu_flush_tlb_group(struct tegra_smmu *smmu,
{
u32 value;
value = SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_ASID(asid) |
SMMU_TLB_FLUSH_VA_GROUP(iova);
if (smmu->soc->num_asids == 4)
value = (asid & 0x3) << 29;
else
value = (asid & 0x7f) << 24;
value |= SMMU_TLB_FLUSH_ASID_MATCH | SMMU_TLB_FLUSH_VA_GROUP(iova);
smmu_writel(smmu, value, SMMU_TLB_FLUSH);
}
......@@ -316,6 +325,9 @@ static void tegra_smmu_domain_free(struct iommu_domain *domain)
/* TODO: free page directory and page tables */
WARN_ON_ONCE(as->use_count);
kfree(as->count);
kfree(as->pts);
kfree(as);
}
......@@ -645,6 +657,7 @@ static int tegra_smmu_map(struct iommu_domain *domain, unsigned long iova,
{
struct tegra_smmu_as *as = to_smmu_as(domain);
dma_addr_t pte_dma;
u32 pte_attrs;
u32 *pte;
pte = as_get_pte(as, iova, &pte_dma);
......@@ -655,8 +668,16 @@ static int tegra_smmu_map(struct iommu_domain *domain, unsigned long iova,
if (*pte == 0)
tegra_smmu_pte_get_use(as, iova);
pte_attrs = SMMU_PTE_NONSECURE;
if (prot & IOMMU_READ)
pte_attrs |= SMMU_PTE_READABLE;
if (prot & IOMMU_WRITE)
pte_attrs |= SMMU_PTE_WRITABLE;
tegra_smmu_set_pte(as, iova, pte, pte_dma,
__phys_to_pfn(paddr) | SMMU_PTE_ATTR);
__phys_to_pfn(paddr) | pte_attrs);
return 0;
}
......
......@@ -388,6 +388,24 @@ int mdev_device_remove(struct device *dev, bool force_remove)
return 0;
}
int mdev_set_iommu_device(struct device *dev, struct device *iommu_device)
{
struct mdev_device *mdev = to_mdev_device(dev);
mdev->iommu_device = iommu_device;
return 0;
}
EXPORT_SYMBOL(mdev_set_iommu_device);
struct device *mdev_get_iommu_device(struct device *dev)
{
struct mdev_device *mdev = to_mdev_device(dev);
return mdev->iommu_device;
}
EXPORT_SYMBOL(mdev_get_iommu_device);
static int __init mdev_init(void)
{
return mdev_bus_register();
......
......@@ -32,6 +32,7 @@ struct mdev_device {
void *driver_data;
struct list_head next;
struct kobject *type_kobj;
struct device *iommu_device;
bool active;
};
......
......@@ -97,6 +97,7 @@ struct vfio_dma {
struct vfio_group {
struct iommu_group *iommu_group;
struct list_head next;
bool mdev_group; /* An mdev group */
};
/*
......@@ -564,7 +565,7 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
mutex_lock(&iommu->lock);
/* Fail if notifier list is empty */
if ((!iommu->external_domain) || (!iommu->notifier.head)) {
if (!iommu->notifier.head) {
ret = -EINVAL;
goto pin_done;
}
......@@ -646,11 +647,6 @@ static int vfio_iommu_type1_unpin_pages(void *iommu_data,
mutex_lock(&iommu->lock);
if (!iommu->external_domain) {
mutex_unlock(&iommu->lock);
return -EINVAL;
}
do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
for (i = 0; i < npage; i++) {
struct vfio_dma *dma;
......@@ -1311,13 +1307,109 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base)
return ret;
}
static struct device *vfio_mdev_get_iommu_device(struct device *dev)
{
struct device *(*fn)(struct device *dev);
struct device *iommu_device;
fn = symbol_get(mdev_get_iommu_device);
if (fn) {
iommu_device = fn(dev);
symbol_put(mdev_get_iommu_device);
return iommu_device;
}
return NULL;
}
static int vfio_mdev_attach_domain(struct device *dev, void *data)
{
struct iommu_domain *domain = data;
struct device *iommu_device;
iommu_device = vfio_mdev_get_iommu_device(dev);
if (iommu_device) {
if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
return iommu_aux_attach_device(domain, iommu_device);
else
return iommu_attach_device(domain, iommu_device);
}
return -EINVAL;
}
static int vfio_mdev_detach_domain(struct device *dev, void *data)
{
struct iommu_domain *domain = data;
struct device *iommu_device;
iommu_device = vfio_mdev_get_iommu_device(dev);
if (iommu_device) {
if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
iommu_aux_detach_device(domain, iommu_device);
else
iommu_detach_device(domain, iommu_device);
}
return 0;
}
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group)
return iommu_group_for_each_dev(group->iommu_group,
domain->domain,
vfio_mdev_attach_domain);
else
return iommu_attach_group(domain->domain, group->iommu_group);
}
static void vfio_iommu_detach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group)
iommu_group_for_each_dev(group->iommu_group, domain->domain,
vfio_mdev_detach_domain);
else
iommu_detach_group(domain->domain, group->iommu_group);
}
static bool vfio_bus_is_mdev(struct bus_type *bus)
{
struct bus_type *mdev_bus;
bool ret = false;
mdev_bus = symbol_get(mdev_bus_type);
if (mdev_bus) {
ret = (bus == mdev_bus);
symbol_put(mdev_bus_type);
}
return ret;
}
static int vfio_mdev_iommu_device(struct device *dev, void *data)
{
struct device **old = data, *new;
new = vfio_mdev_get_iommu_device(dev);
if (!new || (*old && *old != new))
return -EINVAL;
*old = new;
return 0;
}
static int vfio_iommu_type1_attach_group(void *iommu_data,
struct iommu_group *iommu_group)
{
struct vfio_iommu *iommu = iommu_data;
struct vfio_group *group;
struct vfio_domain *domain, *d;
struct bus_type *bus = NULL, *mdev_bus;
struct bus_type *bus = NULL;
int ret;
bool resv_msi, msi_remap;
phys_addr_t resv_msi_base;
......@@ -1352,23 +1444,30 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
if (ret)
goto out_free;
mdev_bus = symbol_get(mdev_bus_type);
if (vfio_bus_is_mdev(bus)) {
struct device *iommu_device = NULL;
if (mdev_bus) {
if ((bus == mdev_bus) && !iommu_present(bus)) {
symbol_put(mdev_bus_type);
group->mdev_group = true;
/* Determine the isolation type */
ret = iommu_group_for_each_dev(iommu_group, &iommu_device,
vfio_mdev_iommu_device);
if (ret || !iommu_device) {
if (!iommu->external_domain) {
INIT_LIST_HEAD(&domain->group_list);
iommu->external_domain = domain;
} else
} else {
kfree(domain);
}
list_add(&group->next,
&iommu->external_domain->group_list);
mutex_unlock(&iommu->lock);
return 0;
}
symbol_put(mdev_bus_type);
bus = iommu_device->bus;
}
domain->domain = iommu_domain_alloc(bus);
......@@ -1386,7 +1485,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
goto out_domain;
}
ret = iommu_attach_group(domain->domain, iommu_group);
ret = vfio_iommu_attach_group(domain, group);
if (ret)
goto out_domain;
......@@ -1418,8 +1517,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
list_for_each_entry(d, &iommu->domain_list, next) {
if (d->domain->ops == domain->domain->ops &&
d->prot == domain->prot) {
iommu_detach_group(domain->domain, iommu_group);
if (!iommu_attach_group(d->domain, iommu_group)) {
vfio_iommu_detach_group(domain, group);
if (!vfio_iommu_attach_group(d, group)) {
list_add(&group->next, &d->group_list);
iommu_domain_free(domain->domain);
kfree(domain);
......@@ -1427,7 +1526,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
return 0;
}
ret = iommu_attach_group(domain->domain, iommu_group);
ret = vfio_iommu_attach_group(domain, group);
if (ret)
goto out_domain;
}
......@@ -1453,7 +1552,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
return 0;
out_detach:
iommu_detach_group(domain->domain, iommu_group);
vfio_iommu_detach_group(domain, group);
out_domain:
iommu_domain_free(domain->domain);
out_free:
......@@ -1544,7 +1643,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
if (!group)
continue;
iommu_detach_group(domain->domain, iommu_group);
vfio_iommu_detach_group(domain, group);
list_del(&group->next);
kfree(group);
/*
......@@ -1610,7 +1709,7 @@ static void vfio_release_domain(struct vfio_domain *domain, bool external)
list_for_each_entry_safe(group, group_tmp,
&domain->group_list, next) {
if (!external)
iommu_detach_group(domain->domain, group->iommu_group);
vfio_iommu_detach_group(domain, group);
list_del(&group->next);
kfree(group);
}
......
......@@ -489,9 +489,11 @@ struct dmar_domain {
/* Domain ids per IOMMU. Use u16 since
* domain ids are 16 bit wide according
* to VT-d spec, section 9.3 */
unsigned int auxd_refcnt; /* Refcount of auxiliary attaching */
bool has_iotlb_device;
struct list_head devices; /* all devices' list */
struct list_head auxd; /* link to device's auxiliary list */
struct iova_domain iovad; /* iova's that belong to this domain */
struct dma_pte *pgd; /* virtual address */
......@@ -510,6 +512,11 @@ struct dmar_domain {
2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
u64 max_addr; /* maximum mapped address */
int default_pasid; /*
* The default pasid used for non-SVM
* traffic on mediated devices.
*/
struct iommu_domain domain; /* generic domain data structure for
iommu core */
};
......@@ -559,6 +566,9 @@ struct device_domain_info {
struct list_head link; /* link to domain siblings */
struct list_head global; /* link to global list */
struct list_head table; /* link to pasid table */
struct list_head auxiliary_domains; /* auxiliary domains
* attached to this device
*/
u8 bus; /* PCI bus number */
u8 devfn; /* PCI devfn number */
u16 pfsid; /* SRIOV physical function source ID */
......@@ -568,6 +578,7 @@ struct device_domain_info {
u8 pri_enabled:1;
u8 ats_supported:1;
u8 ats_enabled:1;
u8 auxd_enabled:1; /* Multiple domains per device */
u8 ats_qdep;
struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
struct intel_iommu *iommu; /* IOMMU used by this device */
......@@ -650,6 +661,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
int for_each_device_domain(int (*fn)(struct device_domain_info *info,
void *data), void *data);
void iommu_flush_write_buffer(struct intel_iommu *iommu);
int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev);
#ifdef CONFIG_INTEL_IOMMU_SVM
int intel_svm_init(struct intel_iommu *iommu);
......@@ -679,7 +691,6 @@ struct intel_svm {
struct list_head list;
};
extern int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev);
extern struct intel_iommu *intel_svm_device_to_iommu(struct device *dev);
#endif
......
......@@ -48,6 +48,7 @@ struct bus_type;
struct device;
struct iommu_domain;
struct notifier_block;
struct iommu_sva;
/* iommu fault flags */
#define IOMMU_FAULT_READ 0x0
......@@ -55,6 +56,8 @@ struct notifier_block;
typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
struct device *, unsigned long, int, void *);
typedef int (*iommu_mm_exit_handler_t)(struct device *dev, struct iommu_sva *,
void *);
struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped */
......@@ -156,6 +159,33 @@ struct iommu_resv_region {
enum iommu_resv_type type;
};
/* Per device IOMMU features */
enum iommu_dev_features {
IOMMU_DEV_FEAT_AUX, /* Aux-domain feature */
IOMMU_DEV_FEAT_SVA, /* Shared Virtual Addresses */
};
#define IOMMU_PASID_INVALID (-1U)
/**
* struct iommu_sva_ops - device driver callbacks for an SVA context
*
* @mm_exit: called when the mm is about to be torn down by exit_mmap. After
* @mm_exit returns, the device must not issue any more transaction
* with the PASID given as argument.
*
* The @mm_exit handler is allowed to sleep. Be careful about the
* locks taken in @mm_exit, because they might lead to deadlocks if
* they are also held when dropping references to the mm. Consider the
* following call chain:
* mutex_lock(A); mmput(mm) -> exit_mm() -> @mm_exit() -> mutex_lock(A)
* Using mmput_async() prevents this scenario.
*
*/
struct iommu_sva_ops {
iommu_mm_exit_handler_t mm_exit;
};
#ifdef CONFIG_IOMMU_API
/**
......@@ -186,6 +216,14 @@ struct iommu_resv_region {
* @of_xlate: add OF master IDs to iommu grouping
* @is_attach_deferred: Check if domain attach should be deferred from iommu
* driver init to device driver init (default no)
* @dev_has/enable/disable_feat: per device entries to check/enable/disable
* iommu specific features.
* @dev_feat_enabled: check enabled feature
* @aux_attach/detach_dev: aux-domain specific attach/detach entries.
* @aux_get_pasid: get the pasid given an aux-domain
* @sva_bind: Bind process address space to device
* @sva_unbind: Unbind process address space from device
* @sva_get_pasid: Get PASID associated to a SVA handle
* @pgsize_bitmap: bitmap of all possible supported page sizes
*/
struct iommu_ops {
......@@ -230,6 +268,22 @@ struct iommu_ops {
int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
/* Per device IOMMU features */
bool (*dev_has_feat)(struct device *dev, enum iommu_dev_features f);
bool (*dev_feat_enabled)(struct device *dev, enum iommu_dev_features f);
int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
/* Aux-domain specific attach/detach entries */
int (*aux_attach_dev)(struct iommu_domain *domain, struct device *dev);
void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm,
void *drvdata);
void (*sva_unbind)(struct iommu_sva *handle);
int (*sva_get_pasid)(struct iommu_sva *handle);
unsigned long pgsize_bitmap;
};
......@@ -392,10 +446,22 @@ struct iommu_fwspec {
const struct iommu_ops *ops;
struct fwnode_handle *iommu_fwnode;
void *iommu_priv;
u32 flags;
unsigned int num_ids;
u32 ids[1];
};
/* ATS is supported */
#define IOMMU_FWSPEC_PCI_RC_ATS (1 << 0)
/**
* struct iommu_sva - handle to a device-mm bond
*/
struct iommu_sva {
struct device *dev;
const struct iommu_sva_ops *ops;
};
int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
const struct iommu_ops *ops);
void iommu_fwspec_free(struct device *dev);
......@@ -416,6 +482,22 @@ static inline void dev_iommu_fwspec_set(struct device *dev,
int iommu_probe_device(struct device *dev);
void iommu_release_device(struct device *dev);
bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features f);
int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features f);
int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev);
void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev);
int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev);
struct iommu_sva *iommu_sva_bind_device(struct device *dev,
struct mm_struct *mm,
void *drvdata);
void iommu_sva_unbind_device(struct iommu_sva *handle);
int iommu_sva_set_ops(struct iommu_sva *handle,
const struct iommu_sva_ops *ops);
int iommu_sva_get_pasid(struct iommu_sva *handle);
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
......@@ -700,6 +782,68 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
return NULL;
}
static inline bool
iommu_dev_has_feature(struct device *dev, enum iommu_dev_features feat)
{
return false;
}
static inline bool
iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features feat)
{
return false;
}
static inline int
iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
{
return -ENODEV;
}
static inline int
iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
{
return -ENODEV;
}
static inline int
iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
{
return -ENODEV;
}
static inline void
iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev)
{
}
static inline int
iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
{
return -ENODEV;
}
static inline struct iommu_sva *
iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
{
return NULL;
}
static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
{
}
static inline int iommu_sva_set_ops(struct iommu_sva *handle,
const struct iommu_sva_ops *ops)
{
return -EINVAL;
}
static inline int iommu_sva_get_pasid(struct iommu_sva *handle)
{
return IOMMU_PASID_INVALID;
}
#endif /* CONFIG_IOMMU_API */
#ifdef CONFIG_IOMMU_DEBUGFS
......
......@@ -76,6 +76,14 @@ struct iova_domain {
unsigned long start_pfn; /* Lower limit for this domain */
unsigned long dma_32bit_pfn;
unsigned long max32_alloc_size; /* Size of last failed allocation */
struct iova_fq __percpu *fq; /* Flush Queue */
atomic64_t fq_flush_start_cnt; /* Number of TLB flushes that
have been started */
atomic64_t fq_flush_finish_cnt; /* Number of TLB flushes that
have been finished */
struct iova anchor; /* rbtree lookup anchor */
struct iova_rcache rcaches[IOVA_RANGE_CACHE_MAX_SIZE]; /* IOVA range caches */
......@@ -85,14 +93,6 @@ struct iova_domain {
iova_entry_dtor entry_dtor; /* IOMMU driver specific destructor for
iova entry */
struct iova_fq __percpu *fq; /* Flush Queue */
atomic64_t fq_flush_start_cnt; /* Number of TLB flushes that
have been started */
atomic64_t fq_flush_finish_cnt; /* Number of TLB flushes that
have been finished */
struct timer_list fq_timer; /* Timer to regularily empty the
flush-queues */
atomic_t fq_timer_on; /* 1 when timer is active, 0
......
......@@ -15,6 +15,20 @@
struct mdev_device;
/*
* Called by the parent device driver to set the device which represents
* this mdev in iommu protection scope. By default, the iommu device is
* NULL, that indicates using vendor defined isolation.
*
* @dev: the mediated device that iommu will isolate.
* @iommu_device: a pci device which represents the iommu for @dev.
*
* Return 0 for success, otherwise negative error value.
*/
int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);
struct device *mdev_get_iommu_device(struct device *dev);
/**
* struct mdev_parent_ops - Structure to be registered for each parent device to
* register the device to mdev module.
......
......@@ -1521,21 +1521,6 @@ static inline void pcie_ecrc_get_policy(char *str) { }
bool pci_ats_disabled(void);
#ifdef CONFIG_PCI_ATS
/* Address Translation Service */
void pci_ats_init(struct pci_dev *dev);
int pci_enable_ats(struct pci_dev *dev, int ps);
void pci_disable_ats(struct pci_dev *dev);
int pci_ats_queue_depth(struct pci_dev *dev);
int pci_ats_page_aligned(struct pci_dev *dev);
#else
static inline void pci_ats_init(struct pci_dev *d) { }
static inline int pci_enable_ats(struct pci_dev *d, int ps) { return -ENODEV; }
static inline void pci_disable_ats(struct pci_dev *d) { }
static inline int pci_ats_queue_depth(struct pci_dev *d) { return -ENODEV; }
static inline int pci_ats_page_aligned(struct pci_dev *dev) { return 0; }
#endif
#ifdef CONFIG_PCIE_PTM
int pci_enable_ptm(struct pci_dev *dev, u8 *granularity);
#else
......@@ -1728,8 +1713,24 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d,
static inline const struct pci_device_id *pci_match_id(const struct pci_device_id *ids,
struct pci_dev *dev)
{ return NULL; }
static inline bool pci_ats_disabled(void) { return true; }
#endif /* CONFIG_PCI */
#ifdef CONFIG_PCI_ATS
/* Address Translation Service */
void pci_ats_init(struct pci_dev *dev);
int pci_enable_ats(struct pci_dev *dev, int ps);
void pci_disable_ats(struct pci_dev *dev);
int pci_ats_queue_depth(struct pci_dev *dev);
int pci_ats_page_aligned(struct pci_dev *dev);
#else
static inline void pci_ats_init(struct pci_dev *d) { }
static inline int pci_enable_ats(struct pci_dev *d, int ps) { return -ENODEV; }
static inline void pci_disable_ats(struct pci_dev *d) { }
static inline int pci_ats_queue_depth(struct pci_dev *d) { return -ENODEV; }
static inline int pci_ats_page_aligned(struct pci_dev *dev) { return 0; }
#endif
/* Include architecture-dependent settings and functions */
#include <asm/pci.h>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment