Re: [PATCH v2 4/4] OvmfPkg/PlatformInitLib: Add PlatformReservationConflictCB

["Gerd Hoffmann" <kraxel@redhat.com>]

  Hi,

> > +/**
> > +  Check whenever the 64bit PCI MMIO window overlaps with a reservation
> > +  from qemu.  If so move down the MMIO window to resolve the conflict.
> > +
> > +  This happens on (virtal) AMD machines with 1TB address space,
> > +  because the AMD IOMMU uses an address window just below 1GB.
> 
> (3) Same two typos, I think, as in the commit message.

Duplicated by the power of cut + paste.

> > +  NewBase = (PlatformInfoHob->PcdPciMmio64Base -
> > +             PlatformInfoHob->PcdPciMmio64Size);
> 
> (6) This appears a typo; we'll want
> 
>   NewBase + PcdPciMmio64Size == E820Entry->BaseAddr

But then NewBase is not aligned.  The assignment above moves it down
while maintaining the existing alignment.

> (9) Do we need any other checks or maybe assertions that we're only
> conflicting with a reserved area, and/or that the subtraction for
> NewBase does not underflow?
> 
> I don't think we can "armor" this very well, I'm just pondering if there
> are any egregious misunderstandings between QEMU and the firmware that
> we might want to catch here. If not, that's OK of course.

Yes, it's hard to design something which can handle all reservations
qemu might do in the future correctly.  And, yes, the code above works
because we know the qemu reservation is smaller than the mmio window, so
moving down to the next naturally aligned address actually solves the
conflict.

take care,
  Gerd