lkml.org 
[lkml]   [2021]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RFC v1 10/11] uapi/virtio-iommu: Add a new request type to send page response
From
Date
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:
>> 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.

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.

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

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

>
>> + __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
>
>> +
>> struct virtio_iommu_req_attach {
>> struct virtio_iommu_req_head head;
>> __le32 domain;
>> --
>> 2.17.1
>>

\
 
 \ /
  Last update: 2021-09-30 11:25    [W:0.119 / U:1.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site