On Wed, Sep 14, 2022 at 02:03:25PM -0600, Alex Williamson wrote:
> > > > + container->pgsizes = 4096;
> > >
> > > This might be a separate question/issue: I wonder if we should use
> > > "sysconf(_SC_PAGE_SIZE)" here instead of 4096.
> > >
> > > With a kernel using a larger page size, e.g. CONFIG_ARM64_64K_PAGES,
> > > the IO page size is likely to be 64K too. If the ioctl fails, this
> > > default 4K setup won't work.
> >
> > Perhaps, but IIRC this solution came about because we originally forgot
> > to expose the IOMMU_INFO flag to indicate the pgsize field was valid.
> > At the time we only supported 4K systems, so it made sense to provide
> > this default, though it is indeed dated.
> >
> > TBH, I don't really see why we should try to continue if the ioctl
> > itself fails, so maybe this should be:
OK. Makes sense to me.
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index ace9562a9ba1..ad188b7649e6 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group,
> > AddressSpace *as,
> > {
> > struct vfio_iommu_type1_info *info;
> >
> > - /*
> > - * FIXME: This assumes that a Type1 IOMMU can map any 64-bit
> > - * IOVA whatsoever. That's not actually true, but the current
> > - * kernel interface doesn't tell us what it can map, and the
> > - * existing Type1 IOMMUs generally support any IOVA we're
> > - * going to actually try in practice.
> > - */
> > ret = vfio_get_iommu_info(container, &info);
> > + if (ret) {
>
> Clearly untested,
>
> ret = -errno;
>
> > + error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
> > + goto enable_discards_exit:;
There is a ":" in-between :)
