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

Re: [PATCH] xen/x86: On x2APIC mode, derive LDR from APIC_ID


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 14 Nov 2023 11:14:22 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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=9aiR8w39VYSpttCJ6qkgGJWuaLfuXf2yNj11oB8BG5M=; b=QvYg/TFqaq0ZvLFHF7QH/1R20acOpKA3IZukj5qjW/enfztkXjgppWz9oesGX9zFZ6bDYx15MBHF2KDlBpRhfb7WD6wiZR43Qf5W+136rdFbHWxNoTSM+JyaADEAARHWnydDi6uW7ovpPV5qAV2jBVRszPflo2DB9sIYuvHSpxjeDgthZW/rfvM97e8c5HF3Y9b/DEGWN8fQuEGOpPkA11uFos6M8kk+DTG5fI/oUsVcKiwCt4UzG+HwRXyj+pbOdzJT8XcnMcfpGtbhwvFgI+GTEtOV42ZlJjYG4nIAuMYranyy+y4uloPU6FtbsTRtZGJ4rzCKR8FLDvwzKFPwNQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eSEDar8s5kJgIKb4CqT95EKEKYg5i5FaDjRDkoAgDuw5drVqlXoe9qE/yBZ7UFho52U/DyHlPl8jzp/V6b7l2ENycZ/v9EYxW+Z+frdYq3cRW50xOMor7NNW/s77rWYr2rmhDejmirFT+1EWj34kmvFBoV5D/bAOu/fUjtObdpWRmw/85i4TxXgk7CPmqIHE2VASDdRawAOrh5Dp/efSy4RtPUgyul9zFR3ftb40wsViShWWVUzOSF7Z+Lqsr03Mbupfkjy1N5rulAYhjfKc4D8g6CpZvbYr8apCnhNngtnffNMqlXhcMVfJyrxUzt4iQxsv5OMexK5alRz6729ALw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Tue, 14 Nov 2023 10:14:57 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 13.11.2023 18:53, Roger Pau Monné wrote:
> On Mon, Nov 13, 2023 at 04:50:23PM +0000, Alejandro Vallejo wrote:
>> Both Intel and AMD manuals agree that on x2APIC mode, the APIC LDR and ID
>> registers are derivable from each other through a fixed formula.
>>
>> Xen uses that formula, but applies it to vCPU IDs (which are sequential)
>> rather than x2APIC_IDs (which are not, at the moment). As I understand it,
>> this is an attempt to tightly pack vCPUs into clusters so each cluster has
>> 16 vCPUs rather than 8, but this is problematic for OSs that might read the
>> x2APIC_ID and internally derive LDR (or the other way around)
> 
> I would replace the underscore from x2APIC ID with a space instead.
> 
> Seeing the commit that introduced the bogus LDR value, I'm not sure it
> was intentional,

Hard to reconstruct over 9 years later. It feels like Alejandro may be right
with his derivation.

> as previous Xen code had:
> 
> u32 id = vlapic_get_reg(vlapic, APIC_ID);
> u32 ldr = ((id & ~0xf) << 16) | (1 << (id & 0xf));
> vlapic_set_reg(vlapic, APIC_LDR, ldr);
> 
> Which was correct, as the LDR was derived from the APIC ID and not the
> vCPU ID.

Well, it gave the appearance of deriving from the APIC ID. Just that it was
missing GET_xAPIC_ID() around the vlapic_get_reg() (hence why LDR was
uniformly 1 on all CPUs).

>> This patch fixes the implementation so we follow the rules in the x2APIC
>> spec(s).
>>
>> While in the neighborhood, replace the u32 type with the standard uint32_t
> 
> Likely wants:
> 
> Fixes: f9e0cccf7b35 ('x86/HVM: fix ID handling of x2APIC emulation')

+1

>> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> 
> I do wonder whether we need to take any precautions with guests being
> able to trigger an APIC reset, and thus seeing a changed LDR register
> if the guest happens to be migrated from an older hypervisor version
> that doesn't have this fix.  IOW: I wonder whether Xen should keep the
> previous bogus LDR value across APIC resets for guests that have
> already seen it.

That earlier change deliberately fixed up any bogus values. I wonder
whether what you suggest will do more good or more harm than going
even farther and once again fixing up bad values in lapic_load_fixup().
After all LDR being wrong affects vlapic_match_logical_addr()'s outcome.
I think one of the two wants adding to the change, though.

Jan



 


Rackspace

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