[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/amd: do not expose HWCR.TscFreqSel to guests
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 13 Sep 2023 10:50:07 +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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=N72GyuoBH7n5uGDp7khnzBdKkWVb/LJ6+Uyvl92gzTA=; b=niGqT8DlQJndR+UusTM7eyphi7sRfuPbhwlvMF510JYUgbTKAXgxU31QBbHyBPADxwkH5XI4UpiE4z2FlFI2NotFleSQvizqHv6RScq9n9AOrAbeqE3nqDkxCuljSEGcdwX6DvUBDE1GD8yc/332XlXPFLYpR/KmPZUbo3+am86JBEoAnWOmLZ5Fn01vpiHcz8/ZT2hm3qmmgriD2G4nHNdmvKmHvZyAU4RNHunOKskCp+1pq9Iy3QsqC45Oy8vgtnihDY+MvTqO9DXvwnQqKi6Bv/jTB3JTlT35GsW+wltcIoeklSoOjGG3oGHsQ9YG2osH70ZaFZfYfoYNPNgsNQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Z6f+hPsFtqOSp5i1hOco+JvxMnr8QqhPmq25ObhWNFNu1ug5vvaUxieBI2TFwtM0IFrOXSoiBTj2sWbSbI59Plw6TQsswLsRLG1gD3+/hfMVEtI+ElOkXHlmPlrqZ5fjOWrjsa9UL3/a0P7aKJAvxt9YKzjoyDTfYaSiZ/sTGQhHmLEgsneLaN5ftadfPtVIp5Lc51/9dyJMr68GDLUNB+SAhIYt2KTWyeYTY1nLMpWa52zNHLkD7txznPjnKYFtDVML68Kl+/PRBU353QoxNJORGroQFUO24n//QMc3e2vJn5pBSFqbptGldzkdz1kSZsSSv1C9hVNoSgP938vZvw==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, solene@xxxxxxxxxxx, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Wed, 13 Sep 2023 08:50:28 +0000
- Ironport-data: A9a23:aMXM0q0mARbtjTmfh/bD5SNwkn2cJEfYwER7XKvMYLTBsI5bpzBTn 2UfCjyPO/feamL8ftkkOY2z8EkF6pDSx9AwGgo/pC1hF35El5HIVI+TRqvS04F+DeWYFR46s J9OAjXkBJppJpMJjk71atANlVEliOfQAOK6UbaYUsxIbVcMYD87jh5+kPIOjIdtgNyoayuAo tq3qMDEULOf82cc3lk8teTb8nuDgNyo4GlD5g1nPqgS1LPjvyJ94Kw3dPnZw0TQGuG4LsbiL 87fwbew+H/u/htFIrtJRZ6iLyXm6paLVeS/oiI+t5qK23CulQRrukoPD9IOaF8/ttm8t4sZJ OOhF3CHYVxB0qXkwIzxWvTDes10FfUuFLTveRBTvSEPpqFvnrSFL/hGVSkL0YMkFulfW21j3 KZJBwI0UDO8xOS94u6AcOk2mZF2RCXrFNt3VnBI6xj8VK9ja7aTBqLA6JlfwSs6gd1IEbDGf c0FZDFzbRPGJRpSJlMQD5F4l+Ct7pX9W2QA9BTJ+uxqsy6KklwZPLvFabI5fvSQQspYhACAr 3/u9GXlGBAKcteYzFJp91r13LWexXOjCd56+LuQ6v97g3O+mG0qUzoSWAbmuMHismeCRIcKQ 6AT0m90xUQoz2SpRNTgWxyzoFafowURHdFXFoUS9wWl2qfSpQGDCQAsVTlFZdornMguSDogz VPPmMnmbRRquaeQQGiQ9Z+Vqy2zIikfKWIeZS4CQhAB6tOlq4Y25jrDQ9NiOK+zkNzuGDv0z iyKrS4xnLEah4gA0KDT1UDKhXegq4bESiYx5x7LRSS14wVhfomnaoe0r1/B4p59wJ2xS1CAu D0OnZiY5eVXVJWVznXTEKMKAa2j4OuDPHvEm1lzEpI99jOrvXm+YYRX5zI4L0BsWioZRQLUj IbokVs5zPdu0LGCNMebv6rZ5xwW8JXd
- Ironport-hdrordr: A9a23:C2nMv6pIIay6YP1ILqh5dT4aV5oieYIsimQD101hICG9E/b5qy nKpp8mPHDP5gr5NEtQ++xoRpPwJU80hKQV3WB5B97LMDUOl1HGEGgI1/qA/9SPIVyaygYUvZ 0LT0A1YOecMWRH
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Wed, Sep 13, 2023 at 08:18:57AM +0200, Jan Beulich wrote:
> On 12.09.2023 18:35, Andrew Cooper wrote:
> > On 12/09/2023 5:23 pm, Roger Pau Monne wrote:
> >> --- a/xen/arch/x86/msr.c
> >> +++ b/xen/arch/x86/msr.c
> >> @@ -240,8 +240,12 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr,
> >> uint64_t *val)
> >> case MSR_K8_HWCR:
> >> if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> >> goto gp_fault;
> >> - *val = get_cpu_family(cp->basic.raw_fms, NULL, NULL) >= 0x10
> >> - ? K8_HWCR_TSC_FREQ_SEL : 0;
> >> + /*
> >> + * OpenBSD 7.3 accesses HWCR unconditionally if the TSC is
> >> reported as
> >> + * Invariant. Do not set TSC_FREQ_SEL as that would trigger
> >> OpenBSD to
> >> + * also poke at PSTATE0.
> >> + */
> >
> > While this is true, the justification for removing this is because
> > TSC_FREQ_SEL is a model specific bit, not an architectural bit in HWCR.
> >
> > Also because it's addition without writing into the migration stream was
> > bogus irrespective of the specifics of the bit.
> >
> > I'm still of the opinion that it's buggy for OpenBSD to be looking at
> > model specific bits when virtualised, but given my latest reading of the
> > AMD manuals, I think OpenBSD *is* well behaved looking at PSTATE0 if it
> > can see TSC_FREQ_SEL.
>
> I'm afraid I'm still at a loss seeing what wording in which PPR makes you
> see a connection. If there was a connection, I'd like to ask that we at
> least properly consider exposing PSTATE0 (or if necessary all PSTATEn) as
> well, with zero value (i.e. in particular with the valid bit clear),
> rather than exposing a r/o bit with the wrong value, upsetting Linux.
But PSTATEn is also non-architectural, so the bit meaning could in
theory change between models.
There seems to be no bit anywhere that signals whether PSTATEn is
available, as it's my reading that you can have PSTATEn without the
architectural PSTATE_{LIMIT,CTRL,STATUS} MSRs that are signaled by
HW_PSTATE CPUID bit.
Thanks, Roger.
|
