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

Re: [PATCH 7/8] x86/mm: make code robust to future PAT changes


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Wed, 7 Dec 2022 12:14:58 +0000
  • Accept-language: en-GB, en-US
  • 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=kfjaaFPr9qgIK24GxosCHUID4h8ezX3uAoK8NINR+es=; b=U7viJugYOCv5uj0ctwogq4WuPlNQptwYmwa+3LtFMs6/ko1e5Cvjumv43xBIdrHYcrOUfmVZ9VnFgem7XacasjmJBt9QZ1OTdOFGMw4NxuuEYwHPyJ/D8LLbT1M6+XLMnE9sOtVUvUa5uaraLu6qSSwWLGEQvy8Yp+h57EOAZ/eYMCmSbTE7dwJrFljaetX5qVRmAOZ2U202h0JOzSnhVwR8+FsifOPwSIWJyQ06nPN0kZZXmJL1yJxPX3lCj328C+7R2cKIU3FSwXWQPds7g/pqno09o/qVfCYNWrrOzxgKiZ9zFbigYgIK/iymFZAWEJPGW+9TAMLGu+VqmVwHxw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mY7jcwyZRDe4L/FUzyc8CtQUYeBq256OkgTokosvr94hnnobbx9SLh4M3WgyaJIXfv0yrdE1Xt+FEeL/GWXL2avndpvbFrsCYQ91W5ZwnufrhF9jPJ4mSurkt6yjNcXF6A7/4VgilMJtaRLL2JY1nx50L5ukE3vXit2e8TELGeO485Zzv1IeSniRvhcwlwogkdjIY7BdphuTBkMPFSL4MiC9URb8v+boFSktze6tPdj9yudMmVZmJUVjA6VrVTMFeNafSKLFXxG/6i/6W//nmVosiyLqzpdyH4KnigFo+Gl2/rPMbVM6ekwyEqFjG2R6rqvXynhSRqwpbJwjC4qD9Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>, "Tim (Xen.org)" <tim@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 07 Dec 2022 12:15:09 +0000
  • Ironport-data: A9a23:COKRFqofxrqROcTsWekjyYTryGpeBmLvZBIvgKrLsJaIsI4StFCzt garIBnTOamDa2r0eo10PNi0ph4O6JXWzt5lTVdv+Cs9EHgQ9JuZCYyVIHmrMnLJJKUvbq7FA +Y2MYCccZ9uHhcwgj/3b9ANeFEljfngqoLUUbKCYWYpAFc+E0/NsDo788YhmIlknNOlNA2Ev NL2sqX3NUSsnjV5KQr40YrawP9UlKm06W1wUmAWP6gR5gaEzyZNVvrzGInqR5fGatgMdgKFb 76rIIGRpgvx4xorA9W5pbf3GmVirmn6ZFXmZtJ+AsBOszAazsAA+v9T2Mk0MC+7vw6hjdFpo OihgLTrIesf0g8gr8xGO/VQO3kW0aSrY9YrK1Dn2SCY5xWun3cBX5yCpaz5VGEV0r8fPI1Ay RAXABM3Y0i52O+S+Ymqd+8vg/k6NebJLZxK7xmMzRmBZRonabbqZvySoPpnhnI3jM0IGuvCb c0EbzYpdA7HfxBEJlYQDtQ5gfusgX78NTZfrTp5p4JuuzSVkFM3jeWraYKFEjCJbZw9ckKwj 2TK5WnmRDodM8SS02Gt+XOwnO7f2yj8Xer+EZXoqaY23QPImgT/DjUnegec+eu0mHeYeMJxO VMV2zItq5Y9oRnDot7VGkfQTGS/lhwWVsdUEuY6wBqQ0aeS6AGcbkAIQztAQN0gqs4tRDYu2 0OJntXmHjhmuvueTnf13rKdtza7IyUWBW4Eey4fTAEB7sXjoYc8lRbGRJBoF6vdpsLxMSH9x XaNtidWr6kSiOYb2qP9+krI6xq8q56MQgMr6wH/WmO+8hg/dIOjf5av61XQ8bBHNonxc7Wal H0Nmszb6f9UC5iIzXSJWL9UROHv4OuZOjrBh1IpB4Mm6zmm53+ke8ZX/S16I0BqdM0DfFcFf XPuhO+Y37cLVFPCUEO9S97Z5xgCpUQ4KenYaw==
  • Ironport-hdrordr: A9a23:iDmVOqxBAXXo2ZExnmYaKrPwKr1zdoMgy1knxilNoH1uA6qlfq WV98jzuiWatN98Yh8dcLO7Scq9qBHnlKKdiLN5Vd3OMDUO3lHYTr2KhrGD/9SPIVybysdtkY tmbqhiGJnRIDFB/KHHCdCDYrMd/OU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHZCSwNNVwbw+XjekWbbMfdFBpGtK5gw/oAgABhoICAAQgcAIAAKvyA
  • Thread-topic: [PATCH 7/8] x86/mm: make code robust to future PAT changes

On 07/12/2022 09:41, Jan Beulich wrote:
> On 06.12.2022 18:55, Demi Marie Obenour wrote:
>> On Tue, Dec 06, 2022 at 12:06:24PM +0000, Andrew Cooper wrote:
>>> On 06/12/2022 04:33, Demi Marie Obenour wrote:
>>>> @@ -961,13 +1000,24 @@ get_page_from_l1e(
>>>>  
>>>>          switch ( l1f & PAGE_CACHE_ATTRS )
>>>>          {
>>>> -        case _PAGE_WB:
>>>> +        default:
>>>> +#ifndef NDEBUG
>>>> +            printk(XENLOG_G_WARNING
>>>> +                   "d%d: Guest tried to use bad cachability attribute %u 
>>>> for MFN %lx\n",
>>>> +                   l1e_owner->domain_id, l1f & PAGE_CACHE_ATTRS, mfn);
>>> %pd.  You absolutely want to convert the PTE bits to a PAT value before
>>> priniting (decimal on a PTE value is useless), and %PRI_mfn.
>> I’ll have to look at the rest of the Xen tree to see how to use this.
>>
>>>> +            pv_inject_hw_exception(TRAP_gp_fault, 0);
>>> As I said on IRC, we do want this, but I'm not certain if we can get
>>> away with just enabling it in debug builds.  _PAGE_GNTTAB was ok because
>>> it has always been like that, but there's a non-trivial chance that
>>> there are existing dom0 kernels which violate this constraint.
>> I chose this approach because it is very simple to implement.  Anything
>> more complex ought to be in a separate patch, IMO.
>>
>>>> +            return -EINVAL;
>>>> +#endif
> As to "complex": Just the "return -EINVAL" would be yet less complex.
> Irrespective of the remark my understanding of Andrew's response is that
> the concern extends to the error return as well.

It's a tradeoff.

From a debugging point of view, #GP is absolutely the way to go, because
you get a full backtrace out in Linux.  It doesn't matter in the
slightest which side of the SYSCALL instruction it points at, because
that's not the interesting information to look at.

Returning -EINVAL stops the problematic cache attributes from being
used, but is far more variable in terms of noticeable change inside
Linux.  It ranges from hitting a BUG(), to having the return code lost.


In this case, I'd err on the side of a #GP fault because it is a
definite error in the guest needing debugging and fixing.  But as I said
originally, it probably needs to be on-by-default with a knob to disable.

~Andrew

 


Rackspace

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