Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 21 Sep 2021 17:16:10 +0100
From:   Jean-Philippe Brucker <jean-philippe@linaro.org>
To:     Vivek Gautam <vivek.gautam@arm.com>
Cc:     linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        iommu@lists.linux-foundation.org,
        virtualization@lists.linux-foundation.org, joro@8bytes.org,
        will.deacon@arm.com, mst@redhat.com, robin.murphy@arm.com,
        eric.auger@redhat.com, kevin.tian@intel.com,
        jacob.jun.pan@linux.intel.com, yi.l.liu@intel.com,
        Lorenzo.Pieralisi@arm.com, shameerali.kolothum.thodi@huawei.com
Subject: Re: [PATCH RFC v1 10/11] uapi/virtio-iommu: Add a new request type
 to send page response
Message-ID: <YUoFSrAK2gi3GWp/@myrica>
References: <20210423095147.27922-1-vivek.gautam@arm.com>
 <20210423095147.27922-11-vivek.gautam@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210423095147.27922-11-vivek.gautam@arm.com>
Precedence: bulk

On Fri, Apr 23, 2021 at 03:21:46PM +0530, Vivek Gautam wrote:
> Once the page faults are handled, the response has to be sent to
> virtio-iommu backend, from where it can be sent to the host to
> prepare the response to a generated io page fault by the device.
> Add a new virt-queue request type to handle this.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
> ---
>  include/uapi/linux/virtio_iommu.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
> index c12d9b6a7243..1b174b98663a 100644
> --- a/include/uapi/linux/virtio_iommu.h
> +++ b/include/uapi/linux/virtio_iommu.h
> @@ -48,6 +48,7 @@ struct virtio_iommu_config {
>  #define VIRTIO_IOMMU_T_PROBE			0x05
>  #define VIRTIO_IOMMU_T_ATTACH_TABLE		0x06
>  #define VIRTIO_IOMMU_T_INVALIDATE		0x07
> +#define VIRTIO_IOMMU_T_PAGE_RESP		0x08
>  
>  /* Status types */
>  #define VIRTIO_IOMMU_S_OK			0x00
> @@ -70,6 +71,23 @@ struct virtio_iommu_req_tail {
>  	__u8					reserved[3];
>  };
>  
> +struct virtio_iommu_req_page_resp {
> +	struct virtio_iommu_req_head		head;
> +	__le32					domain;

I don't think we need this field, since the fault report doesn't come with
a domain.

> +	__le32					endpoint;
> +#define VIRTIO_IOMMU_PAGE_RESP_PASID_VALID	(1 << 0)

To be consistent with the rest of the document this would be
VIRTIO_IOMMU_PAGE_RESP_F_PASID_VALID

> +	__le32					flags;
> +	__le32					pasid;
> +	__le32					grpid;
> +#define VIRTIO_IOMMU_PAGE_RESP_SUCCESS		(0x0)
> +#define VIRTIO_IOMMU_PAGE_RESP_INVALID		(0x1)
> +#define VIRTIO_IOMMU_PAGE_RESP_FAILURE		(0x2)
> +	__le16					resp_code;
> +	__u8					pasid_valid;

This field isn't needed since there already is
VIRTIO_IOMMU_PAGE_RESP_PASID_VALID

> +	__u8					reserved[9];
> +	struct virtio_iommu_req_tail		tail;
> +};

I'd align the size of the struct to 16 bytes, but I don't think that's
strictly necessary.

Thanks,
Jean

> +
>  struct virtio_iommu_req_attach {
>  	struct virtio_iommu_req_head		head;
>  	__le32					domain;
> -- 
> 2.17.1
>