Hi,

> > +    if (iommu_phys_bits && phys_bits > iommu_phys_bits) {
> > +        phys_bits = iommu_phys_bits;
> > +        if (!warned2) {
> > +            warn_report("Using physical bits (%u)"
> > +                        " to prevent VFIO mapping failures",
> > +                        iommu_phys_bits);
> > +            warned2 = true;
> > +        }
> > +    }
> > +
> >      return phys_bits;
> >  }
 
> - As I (may have) mentioned in my OVMF comments, I'm unsure if narrowing
> the VCPU "phys address bits" property due to host IOMMU limitations is a
> good design. To me it feels like hacking one piece of information into
> another (unrelated) piece of information. It vaguely makes me think
> we're going to regret this later. But I don't have any specific, current
> counter-argument, admittedly.

It boils down to:

  (a) do MIN(cpu-phys-bits,iommu-phys-bits) in qemu (this patch)

or

  (b1) communicate iommu-phys-bits to the guest firmware
  (b2) do MIN(cpu-phys-bits,iommu-phys-bits) in the guest.

We certainly had cases cases in the past where taking shortcuts in the
design to simplify things caused problems later on.  So variant (a)
leaves the somewhat ugly feeling that we might regret this some day.

On the other hand switching from (a) to (b) at some point in the future
(should the need arise) shouldn't be much different from doing (b) now.
And the whole phys-bits situation is already messy enough even without
a new iommu-phys-bits setting for the firmware.

So, all in all I think I'm fine with taking this approach.

Acked-by: Gerd Hoffmann <kra...@redhat.com>

take care,
  Gerd


Reply via email to