search for: viommu_domain_finalise

...(void *)prop, len); > + break; > default: > dev_err(dev, "unknown viommu prop 0x%x\n", type); > } > @@ -607,16 +624,23 @@ static struct iommu_domain *viommu_domain_alloc(unsigned type) > return &vdomain->domain; > } > > -static int viommu_domain_finalise(struct viommu_dev *viommu, > +static int viommu_domain_finalise(struct viommu_endpoint *vdev, > struct iommu_domain *domain) > { > int ret; > struct viommu_domain *vdomain = to_viommu_domain(domain); > + struct viommu_dev *viommu = vdev->viommu; > >...

...> I also see a bunch of code like e.g. this: > > > > pg_size = 1UL << __ffs(pgsize_bitmap); > > > > which probably won't DTRT on a 32 bit guest if the bitmap has bits set in the high > > word. > > > > My thought is that in that case viommu_domain_finalise() will fail, do not proceed. That's undefined behaviour in C. You need to make sure this condition is never reached. And spec does not make this illegal at all so it looks like we actually need to handle this gracefully. > > > > > > > + return 0; > > > +} >...

...(void *)prop, len); > break; > + case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK: > + ret = viommu_set_pgsize_bitmap(vdev, (void *)prop, len); > + break; > default: > dev_err(dev, "unknown viommu prop 0x%x\n", type); > } > @@ -615,7 +636,7 @@ static int viommu_domain_finalise(struct viommu_endpoint *vdev, > struct viommu_dev *viommu = vdev->viommu; > struct viommu_domain *vdomain = to_viommu_domain(domain); > > - viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap); > + viommu_page_size = 1UL << __ffs(vdev->pgsize_bitmap); &gt...

...(void *)prop, len); > break; > + case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK: > + ret = viommu_set_pgsize_bitmap(vdev, (void *)prop, len); > + break; > default: > dev_err(dev, "unknown viommu prop 0x%x\n", type); > } > @@ -615,7 +636,7 @@ static int viommu_domain_finalise(struct viommu_endpoint *vdev, > struct viommu_dev *viommu = vdev->viommu; > struct viommu_domain *vdomain = to_viommu_domain(domain); > > - viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap); > + viommu_page_size = 1UL << __ffs(vdev->pgsize_bitmap); &gt...

...> >> I also see a bunch of code like e.g. this: >> >> pg_size = 1UL << __ffs(pgsize_bitmap); >> >> which probably won't DTRT on a 32 bit guest if the bitmap has bits set in the high >> word. >> > > My thought is that in that case viommu_domain_finalise() will fail, do not proceed. > >> >> >>> + return 0; >>> +} >>> + >>> static int viommu_add_resv_mem(struct viommu_endpoint *vdev, >>> struct virtio_iommu_probe_resv_mem *mem, >>> size_t len) >>> @...

...en finalised (not attached to any device, for example) can currently cause an Oops, because we attempt to call ida_free() on ID 0, which may either be unallocated or used by another domain. Only initialise the vdomain->viommu pointer, which denotes a finalised domain, at the end of a successful viommu_domain_finalise(). Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver") Reported-by: Eric Auger <eric.auger at redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org> Reviewed-by: Robin Murphy <robin.murphy at arm.com> Link: https://lore.kernel.org/r/2020032609...

...en finalised (not attached to any device, for example) can currently cause an Oops, because we attempt to call ida_free() on ID 0, which may either be unallocated or used by another domain. Only initialise the vdomain->viommu pointer, which denotes a finalised domain, at the end of a successful viommu_domain_finalise(). Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver") Reported-by: Eric Auger <eric.auger at redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org> Reviewed-by: Robin Murphy <robin.murphy at arm.com> Link: https://lore.kernel.org/r/2020032609...

...en finalised (not attached to any device, for example) can currently cause an Oops, because we attempt to call ida_free() on ID 0, which may either be unallocated or used by another domain. Only initialise the vdomain->viommu pointer, which denotes a finalised domain, at the end of a successful viommu_domain_finalise(). Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver") Reported-by: Eric Auger <eric.auger at redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org> Reviewed-by: Robin Murphy <robin.murphy at arm.com> Link: https://lore.kernel.org/r/2020032609...

...en finalised (not attached to any device, for example) can currently cause an Oops, because we attempt to call ida_free() on ID 0, which may either be unallocated or used by another domain. Only initialise the vdomain->viommu pointer, which denotes a finalised domain, at the end of a successful viommu_domain_finalise(). Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver") Reported-by: Eric Auger <eric.auger at redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org> Reviewed-by: Robin Murphy <robin.murphy at arm.com> Link: https://lore.kernel.org/r/2020032609...

...en finalised (not attached to any device, for example) can currently cause an Oops, because we attempt to call ida_free() on ID 0, which may either be unallocated or used by another domain. Only initialise the vdomain->viommu pointer, which denotes a finalised domain, at the end of a successful viommu_domain_finalise(). Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver") Reported-by: Eric Auger <eric.auger at redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org> Reviewed-by: Robin Murphy <robin.murphy at arm.com> Link: https://lore.kernel.org/r/2020032609...

...en finalised (not attached to any device, for example) can currently cause an Oops, because we attempt to call ida_free() on ID 0, which may either be unallocated or used by another domain. Only initialise the vdomain->viommu pointer, which denotes a finalised domain, at the end of a successful viommu_domain_finalise(). Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver") Reported-by: Eric Auger <eric.auger at redhat.com> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org> Reviewed-by: Robin Murphy <robin.murphy at arm.com> Link: https://lore.kernel.org/r/2020032609...

...(void *)prop, len); > break; > + case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK: > + ret = viommu_set_pgsize_bitmap(vdev, (void *)prop, len); > + break; > default: > dev_err(dev, "unknown viommu prop 0x%x\n", type); > } > @@ -630,7 +647,7 @@ static int viommu_domain_finalise(struct viommu_endpoint *vdev, > > vdomain->id = (unsigned int)ret; > > - domain->pgsize_bitmap = viommu->pgsize_bitmap; > + domain->pgsize_bitmap = vdev->pgsize_bitmap; > domain->geometry = viommu->geometry; > > vdomain->map_flags = viomm...

...h of code like e.g. this: >>> >>> pg_size = 1UL << __ffs(pgsize_bitmap); >>> >>> which probably won't DTRT on a 32 bit guest if the bitmap has bits set in the high >>> word. >>> >> >> My thought is that in that case viommu_domain_finalise() will fail, do not proceed. > > That's undefined behaviour in C. You need to make sure this condition > is never reached. And spec does not make this illegal at all > so it looks like we actually need to handle this gracefully. > > >>> >>> >>>&gt...

..._pgsize_bitmap(vdev, (void *)prop, len); > + break; > default: > dev_err(dev, "unknown viommu prop 0x%x\n", type); > } So given this is necessary early in boot, how about we add this in the config space? And maybe ACPI too ... > @@ -630,7 +647,7 @@ static int viommu_domain_finalise(struct viommu_endpoint *vdev, > > vdomain->id = (unsigned int)ret; > > - domain->pgsize_bitmap = viommu->pgsize_bitmap; > + domain->pgsize_bitmap = vdev->pgsize_bitmap; > domain->geometry = viommu->geometry; > > vdomain->map_flags = viomm...

...pings; @@ -113,6 +117,8 @@ static int viommu_get_req_errno(void *buf, size_t len) return -ENOENT; case VIRTIO_IOMMU_S_FAULT: return -EFAULT; + case VIRTIO_IOMMU_S_NOMEM: + return -ENOMEM; case VIRTIO_IOMMU_S_IOERR: case VIRTIO_IOMMU_S_DEVERR: default: @@ -607,15 +613,15 @@ static int viommu_domain_finalise(struct viommu_dev *viommu, { int ret; struct viommu_domain *vdomain = to_viommu_domain(domain); - unsigned int max_domain = viommu->domain_bits > 31 ? ~0 : - (1U << viommu->domain_bits) - 1; vdomain->viommu = viommu; + vdomain->map_flags = viommu->map_flags;...

...pings; @@ -113,6 +117,8 @@ static int viommu_get_req_errno(void *buf, size_t len) return -ENOENT; case VIRTIO_IOMMU_S_FAULT: return -EFAULT; + case VIRTIO_IOMMU_S_NOMEM: + return -ENOMEM; case VIRTIO_IOMMU_S_IOERR: case VIRTIO_IOMMU_S_DEVERR: default: @@ -607,15 +613,15 @@ static int viommu_domain_finalise(struct viommu_dev *viommu, { int ret; struct viommu_domain *vdomain = to_viommu_domain(domain); - unsigned int max_domain = viommu->domain_bits > 31 ? ~0 : - (1U << viommu->domain_bits) - 1; vdomain->viommu = viommu; + vdomain->map_flags = viommu->map_flags;...

...erent page sizes between physical and virtual endpoints. If there is a way for QEMU to simply reject VFIO devices that don't use the same page mask as what's configured globally, let's do that instead of introducing the page_size_mask property. > > @@ -615,7 +636,7 @@ static int viommu_domain_finalise(struct viommu_endpoint *vdev, > > struct viommu_dev *viommu = vdev->viommu; > > struct viommu_domain *vdomain = to_viommu_domain(domain); > > > > - viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap); > > + viommu_page_size = 1UL << __ffs(vde...

...get_req_errno(void *buf, size_t len) > return -ENOENT; > case VIRTIO_IOMMU_S_FAULT: > return -EFAULT; > + case VIRTIO_IOMMU_S_NOMEM: > + return -ENOMEM; > case VIRTIO_IOMMU_S_IOERR: > case VIRTIO_IOMMU_S_DEVERR: > default: > @@ -607,15 +613,15 @@ static int viommu_domain_finalise(struct viommu_dev *viommu, > { > int ret; > struct viommu_domain *vdomain = to_viommu_domain(domain); > - unsigned int max_domain = viommu->domain_bits > 31 ? ~0 : > - (1U << viommu->domain_bits) - 1; > > vdomain->viommu = viommu; > + vdomain...

...get_req_errno(void *buf, size_t len) > return -ENOENT; > case VIRTIO_IOMMU_S_FAULT: > return -EFAULT; > + case VIRTIO_IOMMU_S_NOMEM: > + return -ENOMEM; > case VIRTIO_IOMMU_S_IOERR: > case VIRTIO_IOMMU_S_DEVERR: > default: > @@ -607,15 +613,15 @@ static int viommu_domain_finalise(struct viommu_dev *viommu, > { > int ret; > struct viommu_domain *vdomain = to_viommu_domain(domain); > - unsigned int max_domain = viommu->domain_bits > 31 ? ~0 : > - (1U << viommu->domain_bits) - 1; > > vdomain->viommu = viommu; > + vdomain...

..._lock); > + vdomain->mappings = RB_ROOT_CACHED; > + > + if (type == IOMMU_DOMAIN_DMA && > + iommu_get_dma_cookie(&vdomain->domain)) { > + kfree(vdomain); > + return NULL; > + } > + > + return &vdomain->domain; > +} > + > +static int viommu_domain_finalise(struct viommu_dev *viommu, > + struct iommu_domain *domain) > +{ > + int ret; > + struct viommu_domain *vdomain = to_viommu_domain(domain); > + unsigned int max_domain = viommu->domain_bits > 31 ? ~0 : > + (1U << viommu->domain_bits) - 1; > + > + vdo...