lkml.org 
[lkml]   [2020]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v4 03/15] iommu/smmu: Report empty domain nesting info
    From
    Date
    Hi Yi,

    On 7/6/20 2:46 PM, Liu, Yi L wrote:
    > Hi Eric,
    >
    >> From: Auger Eric <eric.auger@redhat.com>
    >>
    >> Hi Yi,
    >>
    >> Please add a commit message: instead of returning a boolean for
    >> DOMAIN_ATTR_NESTING, arm_smmu_domain_get_attr() returns a
    >> iommu_nesting_info handle.
    >
    > will do. thanks for the suggestion.
    >
    >>
    >> On 7/4/20 1:26 PM, Liu Yi L wrote:
    >>> Cc: Will Deacon <will@kernel.org>
    >>> Cc: Robin Murphy <robin.murphy@arm.com>
    >>> Cc: Eric Auger <eric.auger@redhat.com>
    >>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
    >>> Suggested-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
    >>> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
    >>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
    >>> ---
    >>> drivers/iommu/arm-smmu-v3.c | 29 +++++++++++++++++++++++++++--
    >>> drivers/iommu/arm-smmu.c | 29 +++++++++++++++++++++++++++--
    >>> 2 files changed, 54 insertions(+), 4 deletions(-)
    >>>
    >>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
    >>> index f578677..0c45d4d 100644
    >>> --- a/drivers/iommu/arm-smmu-v3.c
    >>> +++ b/drivers/iommu/arm-smmu-v3.c
    >>> @@ -3019,6 +3019,32 @@ static struct iommu_group
    >> *arm_smmu_device_group(struct device *dev)
    >>> return group;
    >>> }
    >>>
    >>> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
    >> *smmu_domain,
    >>> + void *data)
    >>> +{
    >>> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
    >>> + u32 size;
    >>> +
    >>> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
    >>> + return -ENODEV;
    >>> +
    >>> + size = sizeof(struct iommu_nesting_info);
    >>> +
    >>> + /*
    >>> + * if provided buffer size is not equal to the size, should
    >>> + * return 0 and also the expected buffer size to caller.
    >>> + */
    >>> + if (info->size != size) {
    >> < size?
    >
    > < size may work as well. but I'd like the caller provide exact buffer size. not sure
    > if it is demand in kernel. do you have any suggestion?

    I just suggested that by analogy with the VFIO argsz


    >
    >>> + info->size = size;
    >>> + return 0;
    >>> + }
    >>> +
    >>> + /* report an empty iommu_nesting_info for now */
    >>> + memset(info, 0x0, size);
    >>> + info->size = size;
    >> For info, the current SMMU NESTED mode is not enabling any nesting. It just forces
    >> the usage of the 2st stage instead of stage1 for single stage translation.
    >
    > yep. The intention is as below:
    >
    > " However it requires changing the get_attr(NESTING) implementations in both
    > SMMU drivers as a precursor of this series, to avoid breaking
    > VFIO_TYPE1_NESTING_IOMMU on Arm. Since we haven't yet defined the
    > nesting_info structs for SMMUv2 and v3, I suppose we could return an empty
    > struct iommu_nesting_info for now?"
    > https://lore.kernel.org/linux-iommu/20200617143909.GA886590@myrica/
    >
    > do you think any other needs to be done for now?

    I understand this is a prerequisite. It was more as an information.
    Returning a void struct is a bit weird but at the moment I don't have
    anything better.

    Thanks

    Eric
    >
    > Regards,
    > Yi Liu
    >
    >> Thanks
    >>
    >> Eric
    >>> + return 0;
    >>> +}
    >>> +
    >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
    >>> enum iommu_attr attr, void *data) { @@ -
    >> 3028,8 +3054,7 @@
    >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
    >>> case IOMMU_DOMAIN_UNMANAGED:
    >>> switch (attr) {
    >>> case DOMAIN_ATTR_NESTING:
    >>> - *(int *)data = (smmu_domain->stage ==
    >> ARM_SMMU_DOMAIN_NESTED);
    >>> - return 0;
    >>> + return arm_smmu_domain_nesting_info(smmu_domain,
    >> data);
    >>> default:
    >>> return -ENODEV;
    >>> }
    >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index
    >>> 243bc4c..908607d 100644
    >>> --- a/drivers/iommu/arm-smmu.c
    >>> +++ b/drivers/iommu/arm-smmu.c
    >>> @@ -1506,6 +1506,32 @@ static struct iommu_group
    >> *arm_smmu_device_group(struct device *dev)
    >>> return group;
    >>> }
    >>>
    >>> +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
    >> *smmu_domain,
    >>> + void *data)
    >>> +{
    >>> + struct iommu_nesting_info *info = (struct iommu_nesting_info *) data;
    >>> + u32 size;
    >>> +
    >>> + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
    >>> + return -ENODEV;
    >>> +
    >>> + size = sizeof(struct iommu_nesting_info);
    >>> +
    >>> + /*
    >>> + * if provided buffer size is not equal to the size, should
    >>> + * return 0 and also the expected buffer size to caller.
    >>> + */
    >>> + if (info->size != size) {
    >>> + info->size = size;
    >>> + return 0;
    >>> + }
    >>> +
    >>> + /* report an empty iommu_nesting_info for now */
    >>> + memset(info, 0x0, size);
    >>> + info->size = size;
    >>> + return 0;
    >>> +}
    >>> +
    >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
    >>> enum iommu_attr attr, void *data) { @@ -
    >> 1515,8 +1541,7 @@
    >>> static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
    >>> case IOMMU_DOMAIN_UNMANAGED:
    >>> switch (attr) {
    >>> case DOMAIN_ATTR_NESTING:
    >>> - *(int *)data = (smmu_domain->stage ==
    >> ARM_SMMU_DOMAIN_NESTED);
    >>> - return 0;
    >>> + return arm_smmu_domain_nesting_info(smmu_domain,
    >> data);
    >>> default:
    >>> return -ENODEV;
    >>> }
    >>>
    >

    \
     
     \ /
      Last update: 2020-07-06 15:22    [W:2.451 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site