On 9/9/20 10:38 AM, Fam Zheng wrote: > On 2020-09-08 20:03, Philippe Mathieu-Daudé wrote: >> This driver uses the host page size to align its memory regions, >> but this size is not always compatible with the IOMMU. Add a >> check if the size matches, and bails out with listing the sizes >> the IOMMU supports. >> >> Example on Aarch64: >> >> $ qemu-system-aarch64 -M virt -drive >> if=none,id=nvme0,file=nvme://0006:90:00.0/1,format=raw >> qemu-system-aarch64: -drive >> if=none,id=nvme0,file=nvme://0006:90:00.0/1,format=raw: Unsupported IOMMU >> page size: 4 KiB >> Available page size: >> 64 KiB >> 512 MiB >> >> Suggested-by: Alex Williamson <alex.william...@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> util/vfio-helpers.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c >> index 55b4107ce69..6d9ec7d365c 100644 >> --- a/util/vfio-helpers.c >> +++ b/util/vfio-helpers.c >> @@ -11,6 +11,7 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "qemu/cutils.h" >> #include <sys/ioctl.h> >> #include <linux/vfio.h> >> #include "qapi/error.h" >> @@ -316,6 +317,25 @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const >> char *device, >> ret = -errno; >> goto fail; >> } >> + if (!(iommu_info.flags & VFIO_IOMMU_INFO_PGSIZES)) { >> + error_setg(errp, "Failed to get IOMMU page size info"); >> + ret = -errno; > > We don't have errno here, do we?
Oops thanks, I'll replace by "ret = -EINVAL;". > >> + goto fail; >> + } >> + if (!extract64(iommu_info.iova_pgsizes, >> + ctz64(qemu_real_host_page_size), 1)) { >> + g_autofree char *host_page_size = >> size_to_str(qemu_real_host_page_size); >> + error_setg(errp, "Unsupported IOMMU page size: %s", host_page_size); >> + error_append_hint(errp, "Available page size:\n"); >> + for (int i = 0; i < 64; i++) { >> + if (extract64(iommu_info.iova_pgsizes, i, 1)) { >> + g_autofree char *iova_pgsizes = size_to_str(1UL << i); >> + error_append_hint(errp, " %s\n", iova_pgsizes); > > Interesting... Since it's printing page size which is fairly low level, > why not just plain (hex) numbers? Because this error is reported to the user (not the developer) and depends of the host/iommu. If you don't mind I'll keep it as it (as the code is written). Thank you for reviewing the series! Phil. > > Fam > >> + } >> + } >> + ret = -EINVAL; >> + goto fail; >> + } >> >> s->device = ioctl(s->group, VFIO_GROUP_GET_DEVICE_FD, device); >> >> -- >> 2.26.2 >> >> >