All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Tim Deegan <tim@xen.org>
Subject: Ping: [PATCH 3/5] x86/PV: drop a few misleading paging_mode_refcounts() checks
Date: Fri, 31 Jul 2020 16:58:30 +0200	[thread overview]
Message-ID: <bc2c4ec4-8703-c7a7-76b6-b79e55bca49e@suse.com> (raw)
In-Reply-To: <9f8d0c4d-dec2-0175-09df-51d5e11c88e1@suse.com>

On 15.07.2020 11:59, Jan Beulich wrote:
> The filling and cleaning up of v->arch.guest_table in new_guest_cr3()
> was apparently inconsistent so far: There was a type ref acquired
> unconditionally for the new top level page table, but the dropping of
> the old type ref was conditional upon !paging_mode_refcounts(). Mirror
> this also to arch_set_info_guest().
> 
> Also move new_guest_cr3()'s #ifdef to around the function - both callers
> now get built only when CONFIG_PV, i.e. no need to retain a stub.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

While I've got an ack from Tim, I think I need either an ack from
Andrew or someone's R-b in order to commit this.

Thanks, Jan

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1122,8 +1122,6 @@ int arch_set_info_guest(
>  
>      if ( !cr3_page )
>          rc = -EINVAL;
> -    else if ( paging_mode_refcounts(d) )
> -        /* nothing */;
>      else if ( cr3_page == v->arch.old_guest_table )
>      {
>          v->arch.old_guest_table = NULL;
> @@ -1144,8 +1142,7 @@ int arch_set_info_guest(
>          case -ERESTART:
>              break;
>          case 0:
> -            if ( !compat && !VM_ASSIST(d, m2p_strict) &&
> -                 !paging_mode_refcounts(d) )
> +            if ( !compat && !VM_ASSIST(d, m2p_strict) )
>                  fill_ro_mpt(cr3_mfn);
>              break;
>          default:
> @@ -1166,7 +1163,7 @@ int arch_set_info_guest(
>  
>              if ( !cr3_page )
>                  rc = -EINVAL;
> -            else if ( !paging_mode_refcounts(d) )
> +            else
>              {
>                  rc = get_page_type_preemptible(cr3_page, PGT_root_page_table);
>                  switch ( rc )
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3149,9 +3149,9 @@ int vcpu_destroy_pagetables(struct vcpu
>      return rc;
>  }
>  
> +#ifdef CONFIG_PV
>  int new_guest_cr3(mfn_t mfn)
>  {
> -#ifdef CONFIG_PV
>      struct vcpu *curr = current;
>      struct domain *d = curr->domain;
>      int rc;
> @@ -3220,7 +3220,7 @@ int new_guest_cr3(mfn_t mfn)
>  
>      pv_destroy_ldt(curr); /* Unconditional TLB flush later. */
>  
> -    if ( !VM_ASSIST(d, m2p_strict) && !paging_mode_refcounts(d) )
> +    if ( !VM_ASSIST(d, m2p_strict) )
>          fill_ro_mpt(mfn);
>      curr->arch.guest_table = pagetable_from_mfn(mfn);
>      update_cr3(curr);
> @@ -3231,30 +3231,24 @@ int new_guest_cr3(mfn_t mfn)
>      {
>          struct page_info *page = mfn_to_page(old_base_mfn);
>  
> -        if ( paging_mode_refcounts(d) )
> -            put_page(page);
> -        else
> -            switch ( rc = put_page_and_type_preemptible(page) )
> -            {
> -            case -EINTR:
> -            case -ERESTART:
> -                curr->arch.old_guest_ptpg = NULL;
> -                curr->arch.old_guest_table = page;
> -                curr->arch.old_guest_table_partial = (rc == -ERESTART);
> -                rc = -ERESTART;
> -                break;
> -            default:
> -                BUG_ON(rc);
> -                break;
> -            }
> +        switch ( rc = put_page_and_type_preemptible(page) )
> +        {
> +        case -EINTR:
> +        case -ERESTART:
> +            curr->arch.old_guest_ptpg = NULL;
> +            curr->arch.old_guest_table = page;
> +            curr->arch.old_guest_table_partial = (rc == -ERESTART);
> +            rc = -ERESTART;
> +            break;
> +        default:
> +            BUG_ON(rc);
> +            break;
> +        }
>      }
>  
>      return rc;
> -#else
> -    ASSERT_UNREACHABLE();
> -    return -EINVAL;
> -#endif
>  }
> +#endif
>  
>  #ifdef CONFIG_PV
>  static int vcpumask_to_pcpumask(
> 
> 



  reply	other threads:[~2020-07-31 14:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-15  9:56 [PATCH 0/5] x86: mostly shadow related XSA-319 follow-up Jan Beulich
2020-07-15  9:58 ` [PATCH 1/5] x86/shadow: dirty VRAM tracking is needed for HVM only Jan Beulich
2020-07-15  9:58 ` [PATCH 2/5] x86/shadow: shadow_table[] needs only one entry for PV-only configs Jan Beulich
2020-07-15  9:59 ` [PATCH 3/5] x86/PV: drop a few misleading paging_mode_refcounts() checks Jan Beulich
2020-07-31 14:58   ` Jan Beulich [this message]
2020-07-31 15:17     ` Ping: " Andrew Cooper
2020-07-15  9:59 ` [PATCH 4/5] x86/shadow: have just a single instance of sh_set_toplevel_shadow() Jan Beulich
2020-07-15 10:00 ` [PATCH 5/5] x86/shadow: l3table[] and gl3e[] are HVM only Jan Beulich
2020-07-18 18:20   ` Tim Deegan
2020-07-20  8:55     ` Jan Beulich
2020-07-20 17:37       ` Tim Deegan
2020-07-18 18:21 ` [PATCH 0/5] x86: mostly shadow related XSA-319 follow-up Tim Deegan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc2c4ec4-8703-c7a7-76b6-b79e55bca49e@suse.com \
    --to=jbeulich@suse.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.