lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BN9PR11MB5276A3AFF358A0E822AD22B08C94A@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Fri, 22 Dec 2023 03:19:19 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: "Liu, Yi L" <yi.l.liu@...el.com>, "joro@...tes.org" <joro@...tes.org>,
	"alex.williamson@...hat.com" <alex.williamson@...hat.com>, "jgg@...dia.com"
	<jgg@...dia.com>, "robin.murphy@....com" <robin.murphy@....com>,
	"baolu.lu@...ux.intel.com" <baolu.lu@...ux.intel.com>
CC: "cohuck@...hat.com" <cohuck@...hat.com>, "eric.auger@...hat.com"
	<eric.auger@...hat.com>, "nicolinc@...dia.com" <nicolinc@...dia.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "mjrosato@...ux.ibm.com"
	<mjrosato@...ux.ibm.com>, "chao.p.peng@...ux.intel.com"
	<chao.p.peng@...ux.intel.com>, "yi.y.sun@...ux.intel.com"
	<yi.y.sun@...ux.intel.com>, "peterx@...hat.com" <peterx@...hat.com>,
	"jasowang@...hat.com" <jasowang@...hat.com>,
	"shameerali.kolothum.thodi@...wei.com"
	<shameerali.kolothum.thodi@...wei.com>, "lulu@...hat.com" <lulu@...hat.com>,
	"suravee.suthikulpanit@....com" <suravee.suthikulpanit@....com>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>, "Duan,
 Zhenzhong" <zhenzhong.duan@...el.com>, "joao.m.martins@...cle.com"
	<joao.m.martins@...cle.com>, "Zeng, Xin" <xin.zeng@...el.com>, "Zhao, Yan Y"
	<yan.y.zhao@...el.com>, "j.granados@...sung.com" <j.granados@...sung.com>
Subject: RE: [PATCH v7 2/9] iommufd: Add IOMMU_HWPT_INVALIDATE

> From: Liu, Yi L <yi.l.liu@...el.com>
> Sent: Thursday, December 21, 2023 11:40 PM
> +
> +int iommufd_hwpt_invalidate(struct iommufd_ucmd *ucmd)
> +{
> +	struct iommu_hwpt_invalidate *cmd = ucmd->cmd;
> +	struct iommu_user_data_array data_array = {
> +		.type = cmd->req_type,
> +		.uptr = u64_to_user_ptr(cmd->reqs_uptr),
> +		.entry_len = cmd->req_len,
> +		.entry_num = cmd->req_num,
> +	};
> +	struct iommufd_hw_pagetable *hwpt;
> +	u32 done_num = 0;
> +	int rc;
> +
> +	if (cmd->req_num && (!cmd->reqs_uptr || !cmd->req_len)) {
> +		rc = -EINVAL;
> +		goto out;
> +	}

miss a check on the __reserved field.

> @@ -323,6 +323,7 @@ union ucmd_buffer {
>  	struct iommu_hwpt_alloc hwpt;
>  	struct iommu_hwpt_get_dirty_bitmap get_dirty_bitmap;
>  	struct iommu_hwpt_set_dirty_tracking set_dirty_tracking;
> +	struct iommu_hwpt_invalidate cache;

this should be in alphabetic order. I gave this comment in v6 too:

https://lore.kernel.org/linux-iommu/BN9PR11MB5276D8406BF08B853329288C8CB4A@BN9PR11MB5276.namprd11.prod.outlook.com/

> +/**
> + * enum iommu_hwpt_invalidate_data_type - IOMMU HWPT Cache
> Invalidation
> + *                                        Data Type
> + * @IOMMU_HWPT_INVALIDATE_DATA_VTD_S1: Invalidation data for
> VTD_S1
> + */
> +enum iommu_hwpt_invalidate_data_type {
> +	IOMMU_HWPT_INVALIDATE_DATA_VTD_S1,
> +};

Defining DATA_VTD_S1 at this point is fine, if there is no usage on
DATA_NONE. But following vtd specific definitions should be moved
to the later vtd specific patches. they are not used by the common
code anyway.

> +
> +/**
> + * enum iommu_hwpt_vtd_s1_invalidate_flags - Flags for Intel VT-d
> + *                                           stage-1 cache invalidation
> + * @IOMMU_VTD_INV_FLAGS_LEAF: The LEAF flag indicates whether only
> the
> + *                            leaf PTE caching needs to be invalidated
> + *                            and other paging structure caches can be
> + *                            preserved.

"indicates whether the invalidation applies to all-levels page structure
cache or just the leaf PTE cache"

> + */
> +enum iommu_hwpt_vtd_s1_invalidate_flags {
> +	IOMMU_VTD_INV_FLAGS_LEAF = 1 << 0,
> +};
> +
> +/**
> + * enum iommu_hwpt_vtd_s1_invalidate_error - Result of invalidation

"hardware error of invalidation"

> + * @IOMMU_HWPT_INVALIDATE_VTD_S1_ICE: Invalidation Completion
> Error, details
> + *                                     refer to 11.4.7.1 Fault Status Register
> + *                                     of VT-d specification.
> + * @IOMMU_HWPT_INVALIDATE_VTD_S1_ITE: Invalidation Time-out Error,
> details
> + *                                     refer to 11.4.7.1 Fault Status Register
> + *                                     of VT-d specification.
> + */
> +enum iommu_hwpt_vtd_s1_invalidate_error {
> +	IOMMU_HWPT_INVALIDATE_VTD_S1_ICE = 1 << 0,
> +	IOMMU_HWPT_INVALIDATE_VTD_S1_ITE = 1 << 1,
> +};
> +
> +/**
> + * struct iommu_hwpt_vtd_s1_invalidate - Intel VT-d cache invalidation
> + *                                       (IOMMU_HWPT_INVALIDATE_DATA_VTD_S1)
> + * @addr: The start address of the addresses to be invalidated. It needs
> + *        to be 4KB aligned.

'of the range'

> + * @npages: Number of contiguous 4K pages to be invalidated.
> + * @flags: Combination of enum iommu_hwpt_vtd_s1_invalidate_flags
> + * @inv_error: One of enum iommu_hwpt_vtd_s1_invalidate_error

'@...error'

> + *
> + * The Intel VT-d specific invalidation data for user-managed stage-1 cache
> + * invalidation in nested translation. Userspace uses this structure to
> + * tell the impacted cache scope after modifying the stage-1 page table.
> + *
> + * Invalidating all the caches related to the page table by setting @addr
> + * to be 0 and @npages to be U64_MAX.

here should clarify that the invalidation applies to device TLB automatically
for VT-d.

> + *
> + * @inv_error is meaningful only if the request is handled by kernel. This
> + * can be known by checking struct iommu_hwpt_invalidate::req_num
> output.
> + * @inv_error only covers the errors detected by hardware after submitting
> the
> + * invalidation. The software detected errors would go through the normal
> + * ioctl errno.
> + */
> +struct iommu_hwpt_vtd_s1_invalidate {
> +	__aligned_u64 addr;
> +	__aligned_u64 npages;
> +	__u32 flags;
> +	__u32 inv_error;
> +};
> +
> +/**
> + * struct iommu_hwpt_invalidate - ioctl(IOMMU_HWPT_INVALIDATE)
> + * @size: sizeof(struct iommu_hwpt_invalidate)
> + * @hwpt_id: HWPT ID of a nested HWPT for cache invalidation
> + * @reqs_uptr: User pointer to an array having @req_num of cache
> invalidation
> + *             requests. The request entries in the array are of fixed width
> + *             @req_len, and contain a user data structure for invalidation
> + *             request specific to the given hardware page table.

Just:

'User pointer to an array of driver-specific cache invalidation requests'

> + * @req_type: One of enum iommu_hwpt_invalidate_data_type, defining
> the data
> + *            type of all the entries in the invalidation request array. It
> + *            should be a type supported by the hwpt pointed by @hwpt_id.
> + * @req_len: Length (in bytes) of a request entry in the request array
> + * @req_num: Input the number of cache invalidation requests in the array.
> + *           Output the number of requests successfully handled by kernel.
> + * @__reserved: Must be 0.
> + *
> + * Invalidate the iommu cache for user-managed page table. Modifications
> on a
> + * user-managed page table should be followed by this operation to sync
> cache.
> + * Each ioctl can support one or more cache invalidation requests in the
> array
> + * that has a total size of @req_len * @req_num.
> + *
> + * An empty invalidation request array by setting @req_num==0 is allowed,
> and
> + * @req_len and @reqs_uptr would be ignored in this case. This can be
> used to
> + * check if the given @req_type is supported or not by kernel.
> + */
> +struct iommu_hwpt_invalidate {
> +	__u32 size;
> +	__u32 hwpt_id;
> +	__aligned_u64 reqs_uptr;
> +	__u32 req_type;
> +	__u32 req_len;
> +	__u32 req_num;
> +	__u32 __reserved;
> +};
> +#define IOMMU_HWPT_INVALIDATE _IO(IOMMUFD_TYPE,
> IOMMUFD_CMD_HWPT_INVALIDATE)
>  #endif
> --
> 2.34.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ