search for: aperture_start

...ging to the IOAPIC regions to map phys addr and this stalled qemu with a drown trace: "virtio: bogus descriptor or out of resources" those regions need to be excluded from the iova allocator. This was resolved by adding if (iommu_dma_init_domain(domain, vdev->viommu->geometry.aperture_start, vdev->viommu->geometry.aperture_end, dev)) in viommu_attach_dev() Thanks Eric > + break; > + } > + > + list_add(&vdev->resv_regions, &region->list); > + > + if (mem->subtype != VIRTIO_IOMMU_RESV_MEM_T_RESERVED && > + mem->subt...

...ging to the IOAPIC regions to map phys addr and this stalled qemu with a drown trace: "virtio: bogus descriptor or out of resources" those regions need to be excluded from the iova allocator. This was resolved by adding if (iommu_dma_init_domain(domain, vdev->viommu->geometry.aperture_start, vdev->viommu->geometry.aperture_end, dev)) in viommu_attach_dev() Thanks Eric > + break; > + } > + > + list_add(&vdev->resv_regions, &region->list); > + > + if (mem->subtype != VIRTIO_IOMMU_RESV_MEM_T_RESERVED && > + mem->subt...

...nge range; > + struct vdpa_iova_range vdpa_range; > + > + if (!ops->set_map && !ops->dma_map) { Why not just check if (ops->get_iova_range) directly? > + iommu_domain_get_attr(v->domain, > + DOMAIN_ATTR_GEOMETRY, &geo); > + range.start = geo.aperture_start; > + range.end = geo.aperture_end; > + } else { > + vdpa_range = ops->get_iova_range(vdpa); > + range.start = vdpa_range.start; > + range.end = vdpa_range.end; > + } > + > + return copy_to_user(argp, &range, sizeof(range)); > + > +} > + > static long...

...nge range; > + struct vdpa_iova_range vdpa_range; > + > + if (!ops->set_map && !ops->dma_map) { Why not just check if (ops->get_iova_range) directly? > + iommu_domain_get_attr(v->domain, > + DOMAIN_ATTR_GEOMETRY, &geo); > + range.start = geo.aperture_start; > + range.end = geo.aperture_end; > + } else { > + vdpa_range = ops->get_iova_range(vdpa); > + range.start = vdpa_range.start; > + range.end = vdpa_range.end; > + } > + > + return copy_to_user(argp, &range, sizeof(range)); > + > +} > + > static long...

On 2020/8/10 ??8:05, Michael S. Tsirkin wrote: > On Thu, Aug 06, 2020 at 03:43:54PM +0300, Eli Cohen wrote: >> On Thu, Aug 06, 2020 at 08:29:22AM -0400, Michael S. Tsirkin wrote: >>> On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote: >>>> On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote: >>>>> On Wed, Jun 17, 2020 at 11:29:44AM

On 2020/8/10 ??8:05, Michael S. Tsirkin wrote: > On Thu, Aug 06, 2020 at 03:43:54PM +0300, Eli Cohen wrote: >> On Thu, Aug 06, 2020 at 08:29:22AM -0400, Michael S. Tsirkin wrote: >>> On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote: >>>> On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote: >>>>> On Wed, Jun 17, 2020 at 11:29:44AM

...+ struct device *dev; + struct virtio_device *vdev; + + struct virtqueue *vq; + struct list_head pending_requests; + /* Serialize anything touching the vq and the request list */ + spinlock_t vq_lock; + + struct list_head list; + + /* Device configuration */ + u64 pgsize_bitmap; + u64 aperture_start; + u64 aperture_end; +}; + +struct viommu_mapping { + phys_addr_t paddr; + struct interval_tree_node iova; +}; + +struct viommu_domain { + struct iommu_domain domain; + struct viommu_dev *viommu; + struct mutex mutex; + u64 id; + + spinlock_t mappings_lock; + struct rb_root mappings...

...ddr and this stalled qemu > with a drown trace: > > "virtio: bogus descriptor or out of resources" > > those regions need to be excluded from the iova allocator. This was > resolved by adding > if (iommu_dma_init_domain(domain, > vdev->viommu->geometry.aperture_start, > vdev->viommu->geometry.aperture_end, > dev)) > in viommu_attach_dev() The most recent hack for x86 [1] does call iommu_dma_init_domain() in attach_dev(). Is it buggy? We probably shouldn't call iommu_dma_init_domain() unconditionally (outside of CONFIG_X86 that is)...

...>vdpa; + const struct vdpa_config_ops *ops = vdpa->config; + struct vhost_vdpa_iova_range range; + struct vdpa_iova_range vdpa_range; + + if (!ops->set_map && !ops->dma_map) { + iommu_domain_get_attr(v->domain, + DOMAIN_ATTR_GEOMETRY, &geo); + range.start = geo.aperture_start; + range.end = geo.aperture_end; + } else { + vdpa_range = ops->get_iova_range(vdpa); + range.start = vdpa_range.start; + range.end = vdpa_range.end; + } + + return copy_to_user(argp, &range, sizeof(range)); + +} + static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cm...

...O_IOMMU_F_PROBE, - struct virtio_iommu_config, probe_size, - &viommu->probe_size); + virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_PROBE, + struct virtio_iommu_config, probe_size, + &viommu->probe_size); viommu->geometry = (struct iommu_domain_geometry) { .aperture_start = input_start, -- MST

On Wed, Jun 17, 2020 at 11:29:46AM +0800, Jason Wang wrote: > In order to let userspace work correctly, get_iova_range() is a must > for the device that has its own DMA translation logic. I guess you mean for a device. However in absence of ths op, I don't see what is wrong with just assuming device can access any address. > > Signed-off-by: Jason Wang <jasowang at

...arguably easier to get wrong than > > getting the size if you need it. > > > Yes, so do you still prefer first/last or just begin/end which is consistent > with iommu_domain_geometry? > > Thanks I prefer first/last I think, these are unambiguous. E.g. dma_addr_t aperture_start; /* First address that can be mapped */ dma_addr_t aperture_end; /* Last address that can be mapped */ instead of addressing ambiguity with a comment, let's just name the field well. > > >

...>get_iova_range) { >> + *range = ops->get_iova_range(vdpa); >> + } else if (v->domain && >> + !iommu_domain_get_attr(v->domain, >> + DOMAIN_ATTR_GEOMETRY, &geo) && >> + geo.force_aperture) { >> + range->first = geo.aperture_start; >> + range->last = geo.aperture_end; >> + } else { >> + range->first = 0; >> + range->last = ULLONG_MAX; >> + } > Shouldn't we require drivers that publish VIRTIO_F_ACCESS_PLATFORM to > implement get_iova_range? Probably not, since ACCESS_PLATF...

...a hint that the device has its own DMA translation logic. Device without get_iova_range does not necessarily meant it use IOMMU driver. Thanks > > > > >> + iommu_domain_get_attr(v->domain, >> + DOMAIN_ATTR_GEOMETRY, &geo); >> + range.start = geo.aperture_start; >> + range.end = geo.aperture_end; >> + } else { >> + vdpa_range = ops->get_iova_range(vdpa); >> + range.start = vdpa_range.start; >> + range.end = vdpa_range.end; >> + } >> + >> + return copy_to_user(argp, &range, sizeof(range)); >>...

Hi All: This series introduces API for reporing IOVA range. This is a must for userspace to work correclty: - for the process that uses vhost-vDPA directly to properly allocate IOVA - for VM(qemu), when vIOMMU is not enabled, fail early if GPA is out of range - for VM(qemu), when vIOMMU is enabled, determine a valid guest address width Please review. Thanks Jason Wang (4): vdpa:

Hi All: This series introduces API for reporing IOVA range. This is a must for userspace to work correclty: - for the process that uses vhost-vDPA directly to properly allocate IOVA - for VM(qemu), when vIOMMU is not enabled, fail early if GPA is out of range - for VM(qemu), when vIOMMU is enabled, determine a valid guest address width Please review. Thanks Jason Wang (4): vdpa:

...> + > + struct virtqueue *vq; > + struct list_head pending_requests; > + /* Serialize anything touching the vq and the request list */ > + spinlock_t vq_lock; > + > + struct list_head list; > + > + /* Device configuration */ > + u64 pgsize_bitmap; > + u64 aperture_start; > + u64 aperture_end; > +}; > + > +struct viommu_mapping { > + phys_addr_t paddr; > + struct interval_tree_node iova; > +}; > + > +struct viommu_domain { > + struct iommu_domain domain; > + struct viommu_dev *viommu; > + struct mutex mutex; > + u64...

...> + > + struct virtqueue *vq; > + struct list_head pending_requests; > + /* Serialize anything touching the vq and the request list */ > + spinlock_t vq_lock; > + > + struct list_head list; > + > + /* Device configuration */ > + u64 pgsize_bitmap; > + u64 aperture_start; > + u64 aperture_end; > +}; > + > +struct viommu_mapping { > + phys_addr_t paddr; > + struct interval_tree_node iova; > +}; > + > +struct viommu_domain { > + struct iommu_domain domain; > + struct viommu_dev *viommu; > + struct mutex mutex; > + u64...

...fig, domain_bits, > &viommu->domain_bits); > > + virtio_cread_feature(vdev, VIRTIO_IOMMU_F_PROBE, > + struct virtio_iommu_config, probe_size, > + &viommu->probe_size); > + > viommu->geometry = (struct iommu_domain_geometry) { > .aperture_start = input_start, > .aperture_end = input_end, > @@ -913,6 +1047,7 @@ static unsigned int features[] = { > VIRTIO_IOMMU_F_MAP_UNMAP, > VIRTIO_IOMMU_F_DOMAIN_BITS, > VIRTIO_IOMMU_F_INPUT_RANGE, > + VIRTIO_IOMMU_F_PROBE, > }; > > static struct virtio_device_id id_...

Hi All: This series introduces API for reporing IOVA range. This is a must for userspace to work correclty: - for the process that uses vhost-vDPA directly to properly allocate IOVA - for VM(qemu), when vIOMMU is not enabled, fail early if GPA is out of range - for VM(qemu), when vIOMMU is enabled, determine a valid guest address width Please review. Changes from V1: - do not mandate