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

Re: [PATCH] x86/viridian: EOI MSR should always happen in affected vCPU context


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 1 Apr 2021 16:41:36 +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:X-MS-Exchange-SenderADCheck; bh=WIBgrf8rWSxNvI4UbkFBbnOvBhTRTt+1mPDmdjgQG8U=; b=Re0IFBc9MLO39ckJWRsSDgVMVKJpYryrLKgBRZW2wgcw210P+TnVgnNoG3BRCe+fd/0V8P+YcjI6LIpV7Wg91KDVItjzst/BlrBqg4ZWkahJxEQOMTlkFt8yyoXGBTaDYWAzO+rMuDAdPFvldOui/K2MnToGArxu0Om4laspWKGKWXiB+/47ozk446cNiYe2jHq5bfL0Rmowle6GK7uEB/HbPmGitizOce0K2qVjcIetBBrch6EqeIX4EapUIWM/m+DCSqlwHEGmxMg4uq5ZoGukL4QZ+Vo7BXFh6SuBtq/r3VYhxu8JSyV1JmcZmwTzu9tBRJlaNlD8SkU8A5lu+w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Onzlnr8I7O9M67fvOGY/bKshC+JpdakDZUfirwu0JVtKy3YgMeWvFADVND25oUF7aWu6HRVNejNnGAig8rVmP0aHp8O2p4Ml1ZK+mU70nXt/2VXILA6Tg5IDxHDRWfG3wVWCYJcFv+sACRwzYaarQNJ9FuxHpJGi7tvuq+720RIcFjfFdCEaAVmbe1fYwJtYbopPPtY+TDitXHl2+hwV/sOugz+Cvu9pYgTwuJxoid0FXdPfxLt7gL8jmSm3vxzgE7w0m1j8DwqP3airddzKTc0DcKH3PQWmrx6ZVgCC/pFn2JOUwW5qr5R26gUc5o79issRUDlz3JOXx4Ik/bTuTw==
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
  • Delivery-date: Thu, 01 Apr 2021 14:41:49 +0000
  • Ironport-hdrordr: A9a23:Hmhh/qkRISdQzE4Hz/+EPcz1h+rpDfOcj2dD5ilNYBxZY6Wkvu iUtrAyyQL0hDENWHsphNCHP+26TWnB8INuiLN9AZ6LZyOjnGezNolt4c/ZwzPmEzDj7eI178 1dWoBEIpnLAVB+5PyQ3CCRGdwt2cTC1aiui/vXwXsFd3ATV4hL6QBlBgGHVmh/QwdbDZQ0fa DsnfZvjTymZHgRc4CHHXEDRefOvJnmk5jhbB4ACXccmUOzpBmv76P3FAXd4wcGX1p0sPwf2E Xmsyi83KWstPmn1gTRvlW+0716kMbso+EzYPCkpdMSLlzX6zqARIMkYLGauSBwnefH0idSrP DpgzMNe/t+8GnQeGbdm2qh5yDF3Cw143HvjX+06EGTxvDRfz4xB8pfiY8xSHKwhCQdlepx36 5R02WSu4A/N2KnoA3G+9PKWxt2/3DEwkYKrO8Jg3RTFasYZbNBxLZvmX99LZYaECr2rL0gCe llZfushsp+TFXyVRDkl1gq5ObpcmU4Hx+ATERHkNeSySJqkHdwyFZd7NADn18bnahNBKVs1q DhCOBFhbtORsgZYeZWH+EaW/a6DWTLXFblLH+SG1L6D6sKUki95aLf0fEQ3qWHaZYIxJw9lN DqS1VDr1M/fEroFImo0IBU9AvOBEGwRy7kxM0bx5URgMy8eJPbdQm4DHw+mcqppPsSRufBXe yoBZ5QC/j/aWT0H4JE2BD/RolSJXESXNZ9gKd5Z3u+5ubwbqH6vO3Sd/jeYJD3Fyw/Z2/5Cn wfGDj/Tf8wrHyDazvdulz8Snntckvw8dZbC67B5dUez4ALK8lJuggRglKp+9GTJVR5w+kLVX o7BImivrKwpGGw82qNxX5uIABhAkFc56ilVWhLqw8MO0b9aq0CpN2bZGBX0BK8V19CZvKTND Qai0V8+KqxIZDV7zslEcibPmWTiGZWuGiHVI4GmqqI5d7sf5QxCppOYt00KSz7UzhO3Sp6om ZKbwEJAnLFHjT1kKO/kdg/H+fEbeRxhw+tPO9ZoX/Srl+nuMkqX3cXNgTeFvK/sEILfX50jk c027IDiLCA8AzfU1cXsaAdChlwT0i5RJhBFx+IYY1InKuDQnAMcU66wRqAix8yfWL28V41nW KJF1zZRdjCHkddtndE0qzj7VNzcSGHc1htb21h2LcNaVjuqzJ91/SGabG01HbUYlwewvsFOD WAejcKJBhyrurHoiK9iXKHFX88wI8pMfGYBLM/c6vL0nfFEvz9qYgWW/tV9o1iLtbgr6sCVv +eYRacKHf9B/ky0wKY4nYjNy8ckghSrdr4nBnk5nO/xngxHL7bJ0lnXagSJ5WE9Hf/Lsz4o6 lRnJYwp6+9I2/xYtmJxeXeaCNCMArapSqzQ/szoZ5ZsKouvNJIbtDmeCqN0GsC0AQ1Lc/ymk 9bWqh97bzbMoJkfsAZeUtijy8UvcXKKFFuvh39A+c4c11okmTSOMmR5aHU7bUoGU+MqWLLSB Gi2jwY++2AWSSN1bQXUf1tZWtXbVUx83Rk8qeJcZbKBACjauFE+x67PxaGAc5gYbnAHa9VqB Bwp8yMlauQcSHz3QjLpzt1Iq5U6Q+cMLGPKRPJHfQN6sCwPFSHn7Cj78GyhirmUDfTUTVnua RVMUgLKtlZgjYsjIcrwjG/R6z+rEUiiUZf61hc5y7Q85nj5nzaE0FAORDYhZsTXSA7CAn2sf j4
  • Ironport-sdr: OmOanxswyiBeYzkZgXhdPixdxdUIhuiYcDszPZBaca1CgrtMsmGNnE5qxpxibWyVLl7xEiw+zN tjgjFSMk344gEVQlZzx/yvZGFL+SjSrc5RM8yM8ykIA/7LSMQOnKW8imxg8pXjacw/hp58Lv9l PjOyPIZmcI88aL7L46ZIen2P9Q5JULXMtz7VMUJ4MDOs7Ye71tkI4ErZ/shEPfCwza9h0Nndjp sU6CcGNpopRdRdq6FcY/MgCKnlMly50pn7NgW7P6GT+5qwLbrF/1fa1JshOt1HgV3LHNZ/pHx7 N4M=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Apr 01, 2021 at 01:44:59PM +0100, Andrew Cooper wrote:
> On 01/04/2021 11:22, Roger Pau Monne wrote:
> > The HV_X64_MSR_EOI wrmsr should always happen with the target vCPU
> > as current, as there's no support for EOI'ing interrupts on a remote
> > vCPU.
> >
> > While there also turn the unconditional assert at the top of the
> > function into an error on non-debug builds.
> >
> > No functional change intended.
> >
> > Requested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/hvm/viridian/synic.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/hvm/viridian/synic.c 
> > b/xen/arch/x86/hvm/viridian/synic.c
> > index 22e2df27e5d..e18538c60a6 100644
> > --- a/xen/arch/x86/hvm/viridian/synic.c
> > +++ b/xen/arch/x86/hvm/viridian/synic.c
> > @@ -79,11 +79,20 @@ int viridian_synic_wrmsr(struct vcpu *v, uint32_t idx, 
> > uint64_t val)
> >      struct viridian_vcpu *vv = v->arch.hvm.viridian;
> >      struct domain *d = v->domain;
> >  
> > -    ASSERT(v == current || !v->is_running);
> > +    if ( v != current && v->is_running )
> > +    {
> > +        ASSERT_UNREACHABLE();
> > +        return X86EMUL_EXCEPTION;
> > +    }
> 
> The original ASSERT() was correct - both of these are easily reachable
> in control domain context.

I'm confused, if it's reachable from control domain context then it
shouldn't be an assert?

> If you want EOI to not be used, you need to raise #GP from it, but that
> in principle breaks introspection which really does write MSRs on the
> guests behalf.

Looking at hvm_msr_write_intercept I see that indeed introspection can
monitor an MSR and defer a write, but AFAICT the postponed write will
be performed in guest vcpu context as part of
hvm_vm_event_do_resume?

Also AFAICT there's no way for a control domain to write to this MSR
ATM, as the allowed list in hvm_load_cpu_msrs doesn't contain
HV_X64_MSR_EOI.

Thanks, Roger.



 


Rackspace

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