Re: [Qemu-devel] [RFC v2 3/8] virtio_iommu: add skeleton

[Jean-Philippe Brucker]

On 06/08/2017 12:09 PM, Bharat Bhushan wrote:
>> From: Eric Auger [mailto:address@hidden
>> Sent: Wednesday, June 07, 2017 9:31 PM
>> To: address@hidden; address@hidden;
>> address@hidden; address@hidden; address@hidden;
>> address@hidden; address@hidden; jean-
>> address@hidden
>> Cc: address@hidden; address@hidden; address@hidden;
>> address@hidden; address@hidden; address@hidden;
>> address@hidden; address@hidden; Bharat Bhushan
>> <address@hidden>
>> Subject: [RFC v2 3/8] virtio_iommu: add skeleton
>>
>> This patchs adds the skeleton for the virtio-iommu device.
>>
>> Signed-off-by: Eric Auger <address@hidden>
>> +static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>> +{
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> +    VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
>> +
>> +    virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
>> +                sizeof(struct virtio_iommu_config));
>> +
>> +    s->vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
>> +                             virtio_iommu_handle_command);
>> +
>> +    s->config.page_sizes = ~((1ULL << 12) - 1);
> 
> This is hardcoded to 4K, Should this be aligned to Host-page size ?

I wonder if we should introduce per-address-space page sizes, to cater
for emulated and VFIO devices being managed by the same IOMMU.

For an emulated device, it seems that the page granularity can be
arbitrary, so maybe TARGET_PAGE_MASK would be more convenient. But for
VFIO, the page granularity is a property of the physical IOMMU.

In kvmtool I instantiate two virtio-iommus for vfio and virtio devices,
so the page size issue hasn't come up, but here things won't work if the
page granularity advertised in config.page_sizes is smaller than the
pIOMMU page size.

Adding address space properties is tricky because they change when
attaching devices, and I wanted to avoid this complication. In nested
mode I have to add one AS state, where the AS is active and properties
are freezed (attaching an incompatible device is then rejected). Maybe
we need to do the same for map/unmap.

A simpler solution (for me, that is), would be to put the greatest page
granularity of all VFIO devices into page_sizes, but it doesn't take
hotplug into account.

Thanks,
Jean