Re: [Xen-devel] [PATCH v4 23/35] OvmfPkg/XenPlatformPei: Rework memory detection

[Anthony PERARD <anthony.perard@citrix.com>]

On Wed, Aug 07, 2019 at 05:34:32PM +0200, Roger Pau Monné wrote:
> On Mon, Jul 29, 2019 at 04:39:32PM +0100, Anthony PERARD wrote:
> > When running as a Xen PVH guest, there is no CMOS to read the memory
> > size from.  Rework GetSystemMemorySize(Below|Above)4gb() so they can
> > work without CMOS by reading the e820 table.
> > 
> > Rework XenPublishRamRegions to also care for the reserved and ACPI
> > entry in the e820 table. The region that was added by InitializeXen()
> > isn't needed as that same entry is in the e820 table provided by
> > hvmloader.
> > 
> > MTRR settings aren't modified anymore, on HVM it's already done by
> > hvmloader, on PVH it is supposed to have sane default. MTRR will need
> > to be done properly but keeping what's already been done by programmes
>                                                               ^ firmware

I've used programmes instead of firmware because in case of PVH, OVMF is
the first firmware to run, libxl (and what ever it called) is what
causes an MTRR to be setup, no firmware are involved in that.

> > +    //
> > +    // Round up the start address, and round down the end address.
> > +    //
> > +    Base = ALIGN_VALUE (Entry->BaseAddr, (UINT64)EFI_PAGE_SIZE);
> > +    End = (Entry->BaseAddr + Entry->Length) & ~(UINT64)EFI_PAGE_MASK;
> > +
> > +    switch (Entry->Type) {
> > +    case EfiAcpiAddressRangeMemory:
> > +      AddMemoryRangeHob (Base, End);
> > +      break;
> > +    case EfiAcpiAddressRangeACPI:
> > +      AddReservedMemoryRangeHob (Base, End, FALSE);
> > +      break;
> > +    case EfiAcpiAddressRangeReserved:
> > +      if (Base < LocalApic && LocalApic < End) {
> 
> Don't you also need to check for equality? In case such region starts
> at Base == LocalApic?
> 
> I guess it doesn't matter that much since this is to workaround a
> specific issue with hvmloader, but I would like to see this sorted out
> in hvmloader so that there's no clash anymore.

Indeed, it doesn't matter to much, so I've been lazy. But Laszlo have
pointed that out as well, so there were going to be a patch to make the
workaround better. But I feel like I'm going to need to repost the
series, so I'm probably going to fix that as well.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel