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

Re: [PATCH v3 9/9] x86/P2M: relax permissions of PVH Dom0's MMIO entries


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 23 Sep 2021 17:15:22 +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=QhwNl3BWRz36UYDGvQyycjsgS94kBKFE4yuwg4hlGqI=; b=bpc03ETd1SJR1ElPvOskjQxvmNOgfgM0WgU4Q2OSHXarkS0FDQuZtfpelLghjRnqOb+g691/DNAit1n0KMj6KxyvO0fmjzR9TWVU+dznKQecbbgC0NhzyUI1ZcN1pfj+mgHwdiLDE2uLgSKXwUZsUwkBRH94fAo+nJkXrNRNWDR5JGvNXz2vp+RR5usB2IPmIdEhR4QeVA/JqP2CRf59aYgrPUwX9Mjoj39TN8+5Jp62Dtx+1ox5drWgLP4h1dpv3WKY+Ti3nknSgT4HWbU0XbiM9CaqZrju4ghHTxm8OOfhTI5/GI8nshCDxVR/9Nthjd8dcjcwsTUIWQ79hhEc8g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ahzHFFMslkj85t5MqmPOEW/gVTBFK5IBdQKik0RB0is2TmhNvRYURppFQG17g01AUFwov9bL8H0ipjSps6LeJw8vqBX60j9tEFi8trKTAswvJm9/VpLS0/tDSMvUaAQBstXzPTT/sRPNXaHmcct3vgPmZp7hEpMVq5koN5fWM/arkIqC21t8XmYecoR37/c5bb8Jafkm+l7Zw6fDUaKyKnSQCSe4Tm5kNUt20jYZlgSv8aL0fZmYZMU4goF5s0kJnzBJcylmHO4W1Sd8zgtOXW5OxQrB3Jgnr8lD6JagG7JfzgE9hN8BlciG8IHjIKZw9oQxg+cDd5g8rFYIyUB8Nw==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>
  • Delivery-date: Thu, 23 Sep 2021 15:15:35 +0000
  • Ironport-data: A9a23:y0hDb60ohCPiHj98WfbD5Q12kn2cJEfYwER7XKvMYLTBsI5bpzVSx zQYXziDOv2JM2v0Loh/a4rn9E9X7JWDxtRrTQBupC1hF35El5HIVI+TRqvS04J+DSFhoGZPt Zh2hgzodZhsJpPkS5PE3oHJ9RGQ74nRLlbHILOCan0ZqTNMEn970EoywbVh2+aEvPDia++zk YKqyyHgEAfNNw5cagr4PIra9XuDFNyr0N8plgRWicJj5TcypFFMZH4rHomjLmOQf2VhNrXSq 9Avbl2O1jixEx8FUrtJm1tgG6EAaua60QOm0hK6V0U+6/TrS+NbPqsTbZIhhUlrZzqhu+gtl dB1sqGJaSR4J6bHl7VMQx5qHHQrVUFG0OevzXmXtMWSywvNcmf2wuUoB0YzVWEa0r8pWycUr 6VecW1TKEDY7w616OvTpu1EnMMsIdOtJIoCknph0SvYHbAtRpWrr6DiuIICgmlq2pkm8fD2e ekgSmRKUUX8eUdiOAs1IcoBrcmumSyqG9FfgA3M/vdmi4TJ9yRu1JD9PdyTfcaFLe1Fk0Ddq m/Y8mDRBhABKMfZ2TeD6mirhOLEgWX8Qo16PL+y++NugVaT7ncOExBQXly+ycRVkWbnBYgZc RZNvHNz8+5iryRHU+URQTWcmCLZuyYbZuNwMLwj4wqCm/vE5DSWUz1soiF6VPQqs8o/RDoP3 1CPns/0CTEHjIB5WU5x5Z/P8mjvY3Z9wXsqIHZeFFpYv4CLTJQb00qXJuuPBpJZmTEc9dvY+ DmMsCF2rLEal8djO06TrA2f3mrESnQkSGcICuTrsoCNslgRiG2NPdXABb3nARBodt3xor6p5 iRspiRmxLpSZaxhbQTUKAn3IJmn5uyeLBrXikN1Ep8q+lyFoiD4J9sLuGkleh04Y67onAMFh meJ4mu9A7cJYBOXgVJfOdrtW6zGM4C6fTgaahwkRoUXOcUgHON21CpveVSRzwjQfLsEy8kC1 WOgWZ/0Vx4yUP0/pBLvHrt1+eJ7l0gWmDKILbimnkvP7FZrTCPMIVvzGADVNb5RAWLtiFi9z uuzwOPQkEgADL2iPHOHmWPRRHhTRUUG6VnNg5U/XsaIIxZ8GXFnDPnUwLg7fJdikbgTneDNl kxRkGcEoLYmrXGYewiMdF55b7bjAcR2oX4hZHR+Nle0wXkzJ42o6f5HJZcweLAm8s1lzOJ1E KZZK5nRXKwXR2SV4SkZYLn8sJdmKEahizWRMnf3ezM4ZZNhGVDEo4e2Ygv1+SASJSOrrs9i8 aa43wbWTMNbFQRvBcrbcty1yFa1sSRPke5+RRKQcNJSZF/t4M5hLCmo1q07JMQFKBPiwDqG1 lnJXUdE9LeV+4JsqYvHn6GJqYutAtBSJEsCEjmJ96uyOAnb4nGnnd1KXtGXcG2PT2jz4qijO 7lYlqmuLP0dkV9WmINgCLI3n7km7t7iqrIGnARpGHLHMwaiBr96eyTU2MBOsutGx6NDuBvwU UWKo4EINbKMMcLjMVgQOAt6MbjTiaBKwmHfvaYvPUH3xC5r577WA0xdMi6FhDFZMLYoYpgux v0suZJO5gGy4vbw3g1qUsyAG7ywE0E9
  • Ironport-hdrordr: A9a23:myKTUahOgZdaV00RANq6ixx7ZXBQX0t13DAbv31ZSRFFG/FwyP rAoB1L73PJYWgqNU3I+ergBEGBKUmskqKdxbNhR4tKOzOWxVdATbsSlrcKpgePJ8SQzJ8+6U 4NSdkaNDS0NykHsS+Y2njILz9D+qj/zEnAv463pB0MPGIaG52IrT0JcjpzencGOjWubqBJcq Z0iPA3wwZJLh8sH7uG7zQ+LqX+juyOsKijTQ8NBhYh5gXLpTS06ITiGxzd+hsFSTtAzZor7G CAymXCl+qemsD+7iWZ+37Y7pxQltek4txfBPaUgsxQDjn3kA6naKloRrXHljEop+OE7kosjb D30lsdFvU2z0mUUnC+oBPr1QWl+DEy60X6wVvdunfnqdyRfkNzN+NxwaZiNjfJ4Uspu99xlI hR2XiCipZRBRTc2Azg+tnhTXhR5wqJiEtntdRWo21UUIMYZrMUh5cY5llpHJAJGz+/wJw7Ed NpENrX6J9tABKnhkjizytSKeGXLzEO9k/seDlHhiXV6UkZoJlB9Tpa+CRF9U1ws67USPF/lq 352+pT5fdzpmJ/V9MIOA47e7rENoX6e2O7DIujGyWVKEg5AQO5l3fW2sR/2Aj4Qu1D8HMN8K 6xJ2+w81RCIn7TNQ==
  • Ironport-sdr: 6RLjGJjVJNrfGEEGs76qjVkrq5UFAU5TMeW4wNVCgr2rP1BKeuHQhqyg6le8WNf9WWuyKjFhG7 Fr0ooO9rmLwdbOWFI9YhIF3vRUdG1kDkZ7A1Q0leE3+NXTq/7j8UMkndhmVC72JwHf02xQYJ2B p+v96Zhx+ibyimhi2KxjspsLWFXGW6gbrL3Vp/rZ11KOyGBVfHrFf3HtwSB8jgNLVzG3vovLAT nBHWxzXMWA9w/2CiKsj5bYMOGTVvkk7GrEp4h7Iq5+IXo+MqaYoFWJdCmXWltD4wMeyRFM9TlW IiJ3x5tHE2nDFtQ305tjnc4c
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Sep 23, 2021 at 02:15:25PM +0200, Jan Beulich wrote:
> On 23.09.2021 13:54, Roger Pau Monné wrote:
> > On Thu, Sep 23, 2021 at 01:32:52PM +0200, Jan Beulich wrote:
> >> On 23.09.2021 13:10, Roger Pau Monné wrote:
> >>> On Tue, Sep 21, 2021 at 09:21:11AM +0200, Jan Beulich wrote:
> >>>> --- a/xen/arch/x86/mm/p2m.c
> >>>> +++ b/xen/arch/x86/mm/p2m.c
> >>>> @@ -1319,6 +1319,18 @@ static int set_typed_p2m_entry(struct do
> >>>>              return -EPERM;
> >>>>          }
> >>>>  
> >>>> +        /*
> >>>> +         * Gross bodge, to go away again rather sooner than later:
> >>>> +         *
> >>>> +         * For MMIO allow access permissions to accumulate, but only 
> >>>> for Dom0.
> >>>> +         * Since set_identity_p2m_entry() and set_mmio_p2m_entry() 
> >>>> differ in
> >>>> +         * the way they specify "access", this will allow the ultimate 
> >>>> result
> >>>> +         * to be independent of the sequence of operations.
> >>>
> >>> Wouldn't it be better to 'fix' those operations so that they can work
> >>> together?
> >>
> >> Yes, but then we should do this properly by removing all abuse of
> >> p2m_access_t.
> > 
> > I'm not sure I follow what that abuse is.
> 
> As was clarified, p2m_access_t should be solely used by e.g.
> introspection agents, who are then the entity responsible for
> resolving the resulting faults. Any other uses (to e.g. merely
> restrict permissions for other reasons) are really abuses.

But some p2m types don't really have a fixed access tied to them, for
example MMIO regions just inherit whatever is the default access for
the domain, which makes all this look slightly weird. If the access
should solely be used by introspection, then each type should have a
fixed access tied to it?

> That
> is, if e.g. a r/o direct-MMIO mapping is needed, this should not
> be expressed as (p2m_mmio_direct, p2m_access_r) tuple, but would
> require a distinct p2m_mmio_direct_ro type.

I guess we would then require a p2m_mmio_direct_ro,
p2m_mmio_direct_rwx and a p2m_mmio_direct_n at least, and ideally we
would need to differentiate the mandatory regions as present in ACPI
tables using yet different types.

Thanks, Roger.



 


Rackspace

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