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

Re: [PATCH 4/5] x86/PV: restrict TLB flushing after mod_l[234]_entry()


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 11 Jan 2021 14:00:20 +0100
  • 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=tEIKH+J3HvFClxe1H90ZDZp+VTvccNYLgzup+gKTAp8=; b=OEt2UhWucRB31pqTx+e5vOQM6V8eou+cqedREIVnmMwwMEtj7ZI2rRHje+i7wOaxzfB86l5M3ueka6Psq+AwD2inUYMeYBvacXygxlc76aAezn4T/y54RNcLWGfYQt3WqRF+S/tl0O1+jFvETpEGsen9INBFs51FTP3QZmXy+BKMRvjwFJRt/d58Lff4bx5nNbw1pKRuxCE147Q1E/Ntucay+qRzo27NdfWeS1dLlQMpUaacGRA+9Yiu7RTaaEpn7PPWQS3/o68Mqxy2g5Srs/WeuNvJXYBV/RQ31NmizHbB4zR6FEyxpKgyEJ0YvORwb43s7xn+749Gc+Leiczf0g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mJ4T5jBIwJi3+ejuSSIHRejPaOQzy1Xc88sUOTTza9507Q4uxfCNrrIWsBRp17zg6VY/KafHp34s1e7/cyRHMo2HCQeUcbkHtTgltoFz8XHY4dj6b0fpyDv1tkr5OH/fIlY5kEj2sDGSr3CgshHR9tizJy1pB2LWGd7DV2bdHpcK4ECciHTAnWeX9wSEwHoEcM+mGpPmwRM4YvpzXhrAbcWaNYHFzRGjFplzbq+4z1pWtgP+VaVGTNmWb901yZD1tZ1TITbg2VslvQJiRyZ6Mh5FLxYBYddn0+UPmv32x5veIoJwuhyQFp6cTTcj+ZM+hBqfCkRcRAkrsAxnBHug2g==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>
  • Delivery-date: Mon, 11 Jan 2021 13:00:32 +0000
  • Ironport-sdr: AIklYBXYcoLuBpG7Q5IYmeW1etHgBNWCLXbtKg0hvAgaBxB6t3jyHxu6Y6MA53qnQ2cWii6rdP ndhfPiCytiPDAcgPy17acXFVFjJCkjATP1jGo7Gnwhk429juSIPMqDFAB3d1d5888Ro95WJEgW edlCp+T6HAf9k6mxt3JZ590FzN1Ya//X0OYVAIG7nJ81bmy4UOiUv2oBOkvqFbrLpwe1rAp5Z3 3TFGUjjHhjhiiygH56jXL5s4I9ij6VHteqmOl00TC74rupqfpnSScZ9+VvSzocXL4qoyNoH3gW mEw=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Nov 03, 2020 at 11:57:33AM +0100, Jan Beulich wrote:
> Just like we avoid to invoke remote root pt flushes when all uses of an
> L4 table can be accounted for locally, the same can be done for all of
> L[234] for the linear pt flush when the table is a "free floating" one,
> i.e. it is pinned but not hooked up anywhere. While this situation
> doesn't occur very often, it can be observed.
> 
> Since this breaks one of the implications of the XSA-286 fix, drop the
> flush_root_pt_local variable again and set ->root_pgt_changed directly,
> just like it was before that change.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

LGTM, so:

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

It would be good however if Andrew can give is opinion also, since he
was the one to introduce such logic, and it's not trivial.

> ---
> While adjusting the big comment that was added for XSA-286 I wondered
> why it talks about the "construction of 32bit PV guests". How are 64-bit
> PV guests different in this regard?
> 
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3903,8 +3903,7 @@ long do_mmu_update(
>      struct vcpu *curr = current, *v = curr;
>      struct domain *d = v->domain, *pt_owner = d, *pg_owner;
>      mfn_t map_mfn = INVALID_MFN, mfn;
> -    bool flush_linear_pt = false, flush_root_pt_local = false,
> -        flush_root_pt_others = false;
> +    bool flush_linear_pt = false, flush_root_pt_others = false;
>      uint32_t xsm_needed = 0;
>      uint32_t xsm_checked = 0;
>      int rc = put_old_guest_table(curr);
> @@ -4054,7 +4053,9 @@ long do_mmu_update(
>                          break;
>                      rc = mod_l2_entry(va, l2e_from_intpte(req.val), mfn,
>                                        cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
> -                    if ( !rc )
> +                    if ( !rc &&
> +                         (page->u.inuse.type_info & PGT_count_mask) >
> +                         1 + !!(page->u.inuse.type_info & PGT_pinned) )

I think adding a macro to encapsulate the added condition would make
the code clearer, maybe PAGETABLE_IN_USE, _LOADED or _ACTIVE?

Thanks, Roger.



 


Rackspace

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