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

Re: [PATCH 2/4] x86/P2M: relax guarding of MMIO entries

  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 1 Sep 2021 10:50:55 +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; bh=e4RLCQgFsrNS291skH1nHoOEX+XScfAtKaNRaZxFwoU=; b=PyKHCnG0INawMV6wHy37ClZw6lei8xXJGZV/8Bt6HfUJIeeqjgPh+IGMIxBTnNfhsYicgXe6NgDXMRMvb0hcLvTButqlBYVSD6AbsJFAlJq4/4/zWCs5raEPEzsvFwrFXAGv9mWoKkWzrADqTDkG135OvmQ47p8JVcBNfYbmScv4BtjdTWu+TDwkhi36kv59GAqfAz2Q/05kWIKqrtUBNzYnBUwSzjLCt09E+QB8zG9ush0EhcGk04e/e8uB8s71VeaI+rsZbQcrEbYZcHFugK2vZYBSr723/Bxllx2TVtChUUfxqtL9hizFfm9l2iNo1EJdmG/N5iwswjAI6vLhxA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QgXrF9bGjjjNkBdYvOBcL7jyF58lasdR/IR/CWctvywq8YtpkdNGUpJHRDqBaZ+fuc8u2PvEOgm6OoyLGdPGA6UALRGwWi3pdT329954L/weVkLqGelvGoFa2yqgsQz5IO+rqwpC/sq+wlHgN4u76SSeasjC17TegLo1utt8RmA7mQoXbXWwi4CE6V1uww+gGIp1qvfuIiLfhGJ6SOd0Rr3YJHdRb5znZa8H3oQBeCoxTL/s1hbM76GD8nlUpjCJbSwZ5rKm1xbuiKZzRLnESWXVT0PjgTULyLHHZv6hAPN3MTxMvmhxt9faXdP4jHY6PHd6zkXs6ee5ApLUxVYc2Q==
  • 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:51:07 +0000
  • Ironport-hdrordr: A9a23:08NuUKD+Jn1i6JflHemV55DYdb4zR+YMi2TDtnoBMCC9F/bzqy nApoV/6faZskdyZJhko6HiBEDiexLhHPxOkO0s1N6ZNWGMhILrFuFfBODZslrd8kPFh4hgPG RbH5SWyuecMbG3t6nHCcCDfeod/A==
  • Ironport-sdr: lgfB78udmYi6OtbUqX306EOhJxFENfODhTtu2W5RrgqhwIDVpIkwcZ5i0F9BhawhweG2BQ9ER5 fTCoqURUoT9+APor9ZoeVdOS1hh+bzAUWIaomuleZgwbbQsj4Tx2Hy/rNafYGbRTVD3tOjN1n4 /1FBLOOk2g439cJfPN55RHnz7kRgMt4VSazqR5/ZAA/0wVFu8VIKMdxrOHkmPsPYOLQnJRSFPK jYsiWniuTbOKGrFxr9KReIknXXbNMuqOVuT1ZvBCVhT1zOOI2ylZo7+7chAr7QR3cOwr0Vo4P4 JE505lO4P5szXlesGlih4fMQ
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Aug 31, 2021 at 05:38:49PM +0200, Jan Beulich wrote:
> On 31.08.2021 17:25, Andrew Cooper wrote:
> > On 31/08/2021 14:26, Jan Beulich wrote:
> >> On 31.08.2021 15:16, Andrew Cooper wrote:
> >>> On 30/08/2021 14:02, Jan Beulich wrote:
> >>>> Further permit "access" to differ in the "executable" attribute. While
> >>>> ideally only ROM regions would get mapped with X set, getting there is
> >>>> quite a bit of work. Therefore, as a temporary measure, permit X to
> >>>> vary. For Dom0 the more permissive of the types will be used, while for
> >>>> DomU it'll be the more restrictive one.
> >>> Split behaviour between dom0 and domU based on types alone cannot
> >>> possibly be correct.
> >> True, but what do you do.
> >>
> >>> DomU's need to execute ROMs too, and this looks like will malfunction if
> >>> a ROM ends up in the region that HVMLoader relocated RAM from.
> >>>
> >>> As this is a temporary bodge emergency bugfix, don't try to be clever -
> >>> just take the latest access.
> >> And how do we know that that's what is going to work?
> > 
> > Because it's the pre-existing behaviour.
> Valid point. But for the DomU case there simply has not been any
> pre-existing behavior. Hence my desire to be restrictive initially
> there.
> >>  We should
> >> strictly accumulate for Dom0. And what we do for DomU is moot for
> >> the moment, until PCI passthrough becomes a thing for PVH. Hence
> >> I've opted to be restrictive there - I'd rather see things break
> >> (and getting adjusted) when this future work actually gets carried
> >> out, than leave things permissive for no-one to notice that it's
> >> too permissive, leading to an XSA.
> > 
> > Restricting execute permissions is something unique to virt.  It doesn't
> > exist in a non-virtualised system, as I and D side reads are
> > indistinguishable outside of the core.
> > 
> > Furthermore, it is inexpressible on some systems/configurations.
> > 
> > Introspection is the only technology which should be restricting execute
> > permissions in the p2m, and only when it takes responsibility for
> > dealing with the fallout.
> IOW are you saying that the calls to set_identity_p2m_entry()
> (pre-dating XSA-378) were wrong to use p2m_access_rw? Because that's
> what's getting the way here.

I did wonder this before, because I saw some messages on a couple of
systems about mappings override, and I'm not sure why we need to use
p2m_access_rw. My first thought was to suggest to switch to use the
default access type for the domain, like set_mmio_p2m_entry does.

I have to admit I'm not sure I see the point of preventing execution,
but it's possible I'm missing something.

Thanks, Roger.



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