[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/4] x86/PVH: de-duplicate mappings for first Mb of Dom0 memory

  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 1 Sep 2021 10:41:19 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=HTMmjGYrRe680AdAYN8pAzhWSq3wfk6ltNwSmWH6CGc=; b=cBKl8tmsheOhZ6k90EizphfhYZKoIy96QmAgxnex2devo6Gi612xcbLzX5N+ePFDh3T/5YFdTxqBXPOSkolQLp1mFJCSXvUSy/6dNCkYWBJekjr24ln+/RMNWdGuMz8nScWPCnDR9J6VQO1w65B/XDCTOVlR9o+vZS4Z1hpyYnE5E8Nss7TzM5ezduI1/VuQkHTYse270w8XwAq8FLrvYoyKpXpSxz+ZQ2mtzP2vW+RhlwuQCjKCvqCc2qg1gYiNkHEXYVs4wQCxqBKMkwfb4NKAXgfnKrVJ6/tIBzCqfBDT43GZViDav4oQGdqpi6Ji0RN0ga5GjZ3hVt3XP5iLGw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NyRfmk6HC3OBebgm7qUPt6LtwhalpKQe73E2e1erW4oqmh9p+x8C3+Wg6b17ZIV5z7OgUs4vG9U/YBemKQgk5OSgDgK/P/+Pn+eCybF1gnUm3WLwEqQUM3J/VqIdA+yr6YJZ5YkR6AyhhPfmXKV3kWa80HX+TNjlW/ROnzTQ5x5Nf+D1ZsBRE0bVGErc30nTd4lYzo5iLhcx1CWWGP1H0Cy3ogwtYowt79VV2/etPaxSVILamVAu6HU0/NeGkafJ806ZxcGTwJppOVVDC+ulKw2T3TEk9B6cbxxqKLEs1Eb5ZDCbvhJi9RZPybZJK8lOpRIaFs8Bpga04CtJsfkgIQ==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 01 Sep 2021 08:41:57 +0000
  • Ironport-hdrordr: A9a23:ZpVxIamoCnYJap/jxnElZ0sjSmfpDfLW3DAbv31ZSRFFG/Fw9/ rCoB3U73/JYVcqKRUdcLW7UpVoLkmyyXcY2+cs1NSZLWzbUQmTXeJfBOLZqlWNJ8SXzIVgPM xbAspD4bPLbGSTjazBkXSF+9RL+qj6zEh/792usEuETmtRGt9dBx8SMHf9LqXvLjM2fqbQEv Cnl6x6jgvlQ1s7ROKhCEIIWuDSzue77q4PMXY9dmcaABDlt0LR1ILH
  • Ironport-sdr: w8v8xFC/VqwLgk6a+nwW7EkKIIa4lKvOrNPcoipjKTx7NGZDsV7H4j/0aBjbpYX3JstoM8G0an ypX4iTeNKuwpu5MGQqzHjzV3kjW2m65//ahbrIOSJdB2950OkDq/3re3qX5sloO8rzKvFlwRby taZY2otOjjBYatG1q3CDWy6yUfLLflfLTuCzZ2GNoXcroJqIz64Zz2J/HZJ1zzqgGSCxRI20YM 5RcSFq457qkUtv1nyAKrjMGwTqMaoCkC/C0DPdIL3LtrA1Ak8U2rxUrhbFzSMTqbzKYw4u0YiY dcQrtYL1L2n2xCSmm02lVZwX
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Aug 31, 2021 at 03:14:06PM +0200, Jan Beulich wrote:

Sorry for the delay, and likely not being of much help right now.

> On 31.08.2021 15:02, Andrew Cooper wrote:
> > On 30/08/2021 14:02, Jan Beulich wrote:
> >> One of the changes comprising the fixes for XSA-378 disallows replacing
> >> MMIO mappings by unintended (for this purpose) code paths.
> > 
> > I'd drop the brackets.  All it does is confuse the sentence.
> > 
> >>  This means we
> >> need to be more careful about the mappings put in place in this range -
> >> mappings should be created exactly once:
> >> - iommu_hwdom_init() comes first; it should avoid the first Mb,
> >> - pvh_populate_p2m() should insert identity mappings only into ranges
> >>   not populated as RAM,
> >> - pvh_setup_acpi() should again avoid the first Mb, which was already
> >>   dealt with at that point.
> > 
> > This really is a mess.  It also seems very fragile.
> So it seems to me.
> > Why is iommu_hwdom_init() separate in the first place?  It only does
> > mappings up to 4G in the first place, and with this change, it is now
> > [1M-4G)
> I guess we'll want to wait for Roger to return to shed some light on
> this.

iommu_hwdom_init should cover from [0, max_pdx], or 4G if max_pdx is
below that.

IIRC first PVH dom0 implementations used to have a behavior more
similar to PV in iommu_hwdom_init, as they would get almost everything
below 4GB that's not RAM identity mapped in the (IOMMU) page tables.

PVH dom0 has since diverged, and now iommu_hwdom_init just identity
maps reserved regions. We could likely move this somewhere else, but
given it's still shared with PV dom0 (by using the same command line
option, dom0-iommu=map-reserved) I think it would just make option
handling more confusing.

One way to simplify things would be to rely on the hardware provided
memory map to correctly have the regions in the low 1M marked as
reserved, so that iommu_hwdom_init would identity map them. Then we
would just need a bit of special handling to duplicate the data in RAM
regions for the low 1M but we could avoid a lot of complexity.  This
however requires trusting the hardware is not missing required regions
in the low 1M.

Another option might be to slightly modify hwdom_iommu_map so that for
PVH it returns true for all non-RAM regions in the low 1M. That would
avoid having to add another loop in pvh_populate_p2m to map those.

Thanks, Roger.



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.