Hi Jean,


On 9/21/21 9:46 PM, Jean-Philippe Brucker wrote:

On Fri, Apr 23, 2021 at 03:21:46PM +0530, Vivek Gautam wrote:

quoted

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 <redacted>
---
  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.

But here we are sending the response which would be consumed by the vfio 
ultimately. In kvmtool, I am consuming this "virtio_iommu_req_page_resp" 
request in the virtio/iommu driver, extracting the domain from it, and 
using that to call the respective "page_response" ops from 
"vfio_iommu_ops" in the vfio/core driver.

Is this incorrect way of passing on the page-response back to the host 
kernel?

But I think this will have to be worked out with the /dev/iommu framework.

quoted

+	__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

Sure, will update this.

quoted

+	__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

Yes, sure will remove this field.

quoted

+	__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.

Will fix this. Thanks a lot for reviewing.

Best regards
Vivek

Thanks,
Jean

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel