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

Re: [PATCH] x86/mm: PGC_page_table is used by shadow code only


  • To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 30 Nov 2022 08:52:01 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=BWkEif/iI22c/TAUGZodiUuNY9qaYUJEnQ/zVbZeWnI=; b=kFudLWNS2Uy6u2/wEAEUHgeP+diUhxJwGhWLdQkP+/FWE5nB2+LSOv3B04qALTqIFZfJd4c+Q2+D6YEhPlp2Vu/DdbAWcXYXRRBAmy3WUpYexMVKk9oDV0J8BPviBQ4VerTvBhOV8pbISsKufxxJFyqL2hcmuzxG2vf5hFdyC0Pg2p4t2aDN05gIbRdLJaAngh5VebFwimeMwgIi4F0lWMv0wKhylTmiPN4WPFK+A7DIaNopxGFnjQlNkllvBkdM2wqIAlUMr1tLAOltShM5r8JMSmb/DwnaCWWmlsN9ZKHZU5semx9xgSyFHuGfMAqXSBTlwK+edsrtyfvHkrKMmQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TfOSLqzt4SWYpmZvOPaM+jItZHVdmI7E/WJdzRaGmssF/wXxQW52hHzxk+DLWN56x7LMwVNtIg7l8kn7BGDc2T8HHe9hOS5rM/lyK9CrsgnvrF9IqVFVlHNMKUHy3juqwcCsJxdqxLWTWzwXayCZPad4F45JY9HfNR7hKUGXVzorrUoefwlWl/NT5G+75MNzHPEzK5wv/XNB9slykxJ5Q11aVwNGQB7pCvZcQGtadTSGLfEw3dbHmJKje8hWP7LOR1O7+6noiwnVXFEoi9MTHQL+C/EM3G5VMhHpjjP3w7YjOWCmHXs0at0l+3giSHwYoanCem1mJDh7n/I4soQyKg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 30 Nov 2022 07:52:13 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 29.11.2022 21:56, Andrew Cooper wrote:
> On 29/11/2022 14:55, Jan Beulich wrote:
>> By defining the constant to zero when !SHADOW_PAGING we give compilers
>> the chance to eliminate a little more dead code elsewhere in the tree.
>> Plus, as a minor benefit, the general reference count can be one bit
>> wider. (To simplify things, have PGC_page_table change places with
>> PGC_extra.)
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Ahead of making this change, can we please rename it to something less
> confusing, and fix it's comment which is wrong.
> 
> PGC_shadowed_pt is the best I can think of.

Can do, sure.

>> ---
>> tboot.c's update_pagetable_mac() is suspicious: It effectively is a
>> no-op even prior to this change when !SHADOW_PAGING, which can't be
>> quite right. If (guest) page tables are relevant to include in the
>> verification, shouldn't this look for PGT_l<N>_page_table as well? How
>> to deal with HAP guests there is entirely unclear.
> 
> Considering the caller, it MACs every domheap page for domains with
> CDF_s3_integrity.
> 
> The tboot logical also blindly assumes that any non-idle domain has an
> Intel IOMMU context with it.  This only doesn't (trivially) expose
> because struct domain_iommu is embedded in struct domain (rather than
> allocated separately), and reaching into the wrong part of the arch
> union is only mitigated by the tboot logic not being invoked on
> non-intel systems.  (Also the idle domain check is useless, given that
> it's in a for_each_domain() loop).
> 
> It does look a little like the caller is wanting to MAC all Xen data
> that describes the guest, but doing this unilaterally for all shadowed
> guests seems wrong beside the per-domain s3_integrity setting.

Question is - do we care about addressing this (when, as said, it's
unclear how to deal with HAP domains; maybe their actively used p2m
pages would need including instead)? Or should we rather consider
ripping out tboot support?

Jan



 


Rackspace

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