[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: Roger Pau Monne <roger.pau@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Thu, 1 Apr 2021 13:44:59 +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=q94WzQ6SuSuvqMAlhYZcl9EGVF426lyxhYZHeF9slu8=; b=ES4t/n7FS3L1eIKZRG6VS3qluRtcL5+RRyRWb5vUxcONKTtyCcwWnw3DsEkX842a/oIG+N0Nl3oRuBhMOEli0laWkQSnq0pgKTNf9d+2wdXYKJO/wFZwlE/3Ys98QI2mATsdvwbJA82qAG7QqPeqV6eCaY32BkSa4YMaigSWMlYuEhUzvUkERJEJgAHsg7O8JtyWnMFJHbsrDZzhg4RzEC7zX9EwKU5RbOS8cnZFyIP45MCm5OduXyOuBo1mJMLcjVjKBcAgNS0lgMlJoAR6vC7qy69MKiNCSAPGqJBNDggH2t6Kc9HfPgdoyiKvJGrwz0rT7HRXC1zWl4g9R10UVw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fFNBtywWSXAM+eXbihiPCOYpRZDUvU1ttuFzCge3N0swjvYFQ4+kj128VGamAFwfCm9MRkfynSyqCUhuO5gDma/4gIBjgGptJ2F3EuQswui+QOW7hbI4EwVkwZOqOSkjgeJLs5kk+KmPH4sw+pzYUmjsnSsFsYomg652bF0G2YATDZYfOr5DhKMz6ZZstVV62awZmlM0iyc3w9auANt029Cf4EMzf9agBbLut99ln8Z3B5FTnEm/85ULT5Exc4Pwkj/HvMMq3jOkN/lWSWayjhJDMUMKmV2iSCVzmT7W0h1XQxXMbXII9XJX2gmgboShfXR3u54SaLjlntYoJFnk5g==
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Paul Durrant <paul@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
  • Delivery-date: Thu, 01 Apr 2021 12:45:12 +0000
  • Ironport-hdrordr: A9a23:A2XP86jwXyJdBhGzcQ+oA2MjxXBQX3Vw3DAbvn1ZSRFFG/Gwv/ uF2NwGyB75jysQUnk8mdaGfJKNW2/Y6IQd2+csFJ+Ydk3DtHGzJI9vqbHjzTrpBjHk+odmup tIW5NVTOf9BV0St6rHySGlDtctx8SG+qi0heHYi0xgVx1udrsI1WdEIyywe3cGIDVuL5w/CZ aa+45jrz2vZXwYYq2Adwc4dsLEoMDGk4+jXAUPAAQp5BLLoTSj7rP7FBbw5GZgbxpkx7A+/W /Z1zHo/6nLiYDG9jbw9U/2q65Xltzo18dZCKW36/Q9Bz3whm+TFf9ccpKYujRdmpDI1H8Ll5 32rw4kL4BP7RrqDxyIiD/M/yWl7zo08X/lzjaj8AneiOj0XigzBcYEpa8xSGqg12MasNtx0L 1G0gui3vI9Z36w/1Welqz1fipnmUaurX0pnfR7tQ05baIkZKJMtotaxUtJEf47bVHHwbo6G+ pjBty03ocuTXqmaRnizwxS6eC3Um92NhmLRVVqgL3u7xFm2Fp9z0ce2fUFmGYB+J8XW/B/lp T5G5Utu7dUQsAMa6VhQM8HXMusE2TIBSnBKWSIPD3cZe86EkOIj6SyzKQ+5emsdpBN5JwumK 7ZWFcdkWIpYUrhBeCHwZUjyGGNfEyNGRDWju1O7ZlwvbPxAJDxNzeYdVwom8y8590CH8zyQZ +ISdBrKs6mCVGrNZdC3gX4VZUXA2IZStcpttEyXE/Lit7XK7ftqvfQfJ/oVfnQOAdhflm6Lm oIXTD1KskFxFusQGXEjB/YXG6oVVf4+b52DajG78kewIUALeR3w0wooGX8wvvOBSxJs6Qwck c7CqjgiLmHqW6/+nuNz2gBAGsbMm9lpJHbF19arw4DNE35NZwZvc+ERGxU1HybYjt2T8bcFh 9jt016kJjHaaC49GQHMZaKI2iah3wcqDahVJEHgJCO4s/jZ9ceAos5XrdyUSHGDQZ8lwoviG orUn5FembvUhfVzYm1hp0dA+/SM/Nmhh2wHMJSoXXD8WOGpc8uQXMfdyW0UdGehDsvQzY8vC w1z4YvxJ673Rq/I2o2h+o1dHdWbn6MPb5ABAOZILlPlqvTYwF2R2eSjTm8gxU+E1Carnk6ty jEF2m5aPvLCl1StjR93rzx+F15TGmbYnl9c2t3q4F7CGTAtEtiyOPjXNvH70KhLn85hs0NOj DMZjUfZjljwN26zza5sjePH3dO/ORiAsXtSJAYN53D0HKkL4OF0ZwcF/hP5ZB/KZTFqekQS9 +SfAeTMRL1A+4kwBauu34gISV4wUNUyc/A6VnA1iyVzXQ/Cf3dLBBaXLkdOcib9HWhaPCS0p l15OhF9deYAyHUUJqhxq7WZTIYdU+Wjm6yUu0yqZdb+Yg1r6B+GpHHUT3OkFFLtS9OWvvcpQ c7euBc5ruEB6pEO+o1UAhd9kAylNuOIFAw2zaGSNMWTBUItTvjI9iN47D0srIhDU2KmRvoNT Ckglpg1saAexHG6KUTBK0xK1lHcUQQ6Hxt++WZao3bYT/aPt1rzR6fMnWndqVaR7XAMbIMrg xi69XgpZ7aSwPInCTRtyB8OKRA7iKORt6zGhuFHapt/8ahMVqBxous78jbtka5dRKLL2AZj5 ZCb0oec4BqjSQjlpQ+1myKcZPMy3hV2Gd20HVAjV7i2o+v/WfdEwVnCGTi8+RrdAgWFGOJg8 TD+fWfz1Ln7lF+qML+KHs=
  • Ironport-sdr: 6OLotxVhkGUcDYlTwbbbwcs0wWb1nZNCXZrpc8tJjev88GUED73isIiTb9aNVQeZSZxgN9mxmS AY2EbKXDybnr3/OP6+Trf1q1x4o7oKsk1DrAiUZE8iqFYJnjdtnx2sYNjZO2ipKjoeB3XmITdB jKwfHvH/V0X2nHNH3S3pMtLiTYJWvtvNrBe6c9MBrrpDPsGJXFLTjYzbslVF4W2tQC5ON+rT4A vJQGEVTPdfRpVxgEVffQGJq4+7Yub0zrwpuszPS4s3msb1PR1YFOoq5zSe/h1mM8CuNgrpcRcJ ZSU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.

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.

It's perhaps fine in principle to leave that problem to whomever first
wants to poke this MSR from introspection context, but the
ASSERT_UNREACHABLE()s need dropping whatever the introspection angle.

~Andrew




 


Rackspace

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