Received-SPF: pass (google.com: domain of linux-kernel+bounces-94764-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <9c781386-e359-42fb-b3db-4b781508c7da@intel.com>
Date: Thu, 7 Mar 2024 13:03:51 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 10/16] KVM: x86/mmu: Don't force emulation of L2 accesses
 to non-APIC internal slots
Content-Language: en-US
To: Sean Christopherson <seanjc@google.com>, Paolo Bonzini
	<pbonzini@redhat.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Yan Zhao
	<yan.y.zhao@intel.com>, Isaku Yamahata <isaku.yamahata@intel.com>, "Michael
 Roth" <michael.roth@amd.com>, Yu Zhang <yu.c.zhang@linux.intel.com>, "Chao
 Peng" <chao.p.peng@linux.intel.com>, Fuad Tabba <tabba@google.com>, "David
 Matlack" <dmatlack@google.com>
References: <20240228024147.41573-1-seanjc@google.com>
 <20240228024147.41573-11-seanjc@google.com>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240228024147.41573-11-seanjc@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UWpJUUp4SUVPUjlSNjZzbzFaZ2JaLzkwUE5GdGt6bTlsQ0FEblpWRUZSVFNh?=
 =?utf-8?B?QnZEaitBRkF4bVVUb2c2WjNyV2xrRjZucW5zd0hIOE9hNkNmTjZ4c0RSZnhz?=
 =?utf-8?B?L3E4bU9JeUxmU2U2Y3FDR2VMaWdwMkNEWEFDTWlsVkhISDdVWXA3ZVNoNGNh?=
 =?utf-8?B?WmQxaEM5dENKQTBZNE5NOG1keHZodEpwTUpUU1lqdDB0cWpWeWp1elhmTHVz?=
 =?utf-8?B?REtzSDZwVE45N2FyM1BveFFhZDdlVmFFeVIrZ2I0YnIvL0d5U2t2cDZFazFm?=
 =?utf-8?B?QWk1akUzbHRpbUVwaGFldVlXMVA4MnJXN2liTVF5ZmMwS0ZxK29qanZuV0xO?=
 =?utf-8?B?NE45N0dPbEZ1MzNxaGQ0MFJlMG9XWHZuTXYvWjYxekxweUNsTGVqenVMWW5V?=
 =?utf-8?B?NEpENjg1RUYwNHQrenI5cHo1SE5HZkdnWmFlTzBtRUpPWFJEZ2JDT2pTRjJi?=
 =?utf-8?B?ZVVCOWlUU28wWklvVnkyS2FwL1h4TFZSWVp5aGFFMWxXMWVCZ1d2T285enU5?=
 =?utf-8?B?OHkxWGZKMTUvK1dVNEhzWkxUVmRkaG5ReDFmQTV4OVZseWx1Mkl1RXA5dFNC?=
 =?utf-8?B?cnFWV3g0ekFoNldkSzJBem84U0d6NWJYWDZURDRBekZBZXkrN1lUSW5GR0pN?=
 =?utf-8?B?aEtzVVRjV1Y5NmRVMEt3T3VuMHNjMWNSaWJCR0RaMW1BbkpKbjlkdU9KTXpp?=
 =?utf-8?B?d1NPV1Z3VzNveTJmc2FLRTFVcFdnamxSdUh1OXFkNHlPVGIxOWd5QlZUOEJw?=
 =?utf-8?B?OVpjQlhCK1lqSmwzREF1bnMwNTdkYmxkMW95V3Q2VzRQdzN3U1l2eUJPSFZG?=
 =?utf-8?B?dU8xMnBQdFpjV3daYUdJMkJkaVlnOUFRY1V1bEo0dUJMNXF0dG9ibGF6TFg0?=
 =?utf-8?B?YjBLcGNmU2lHM0ZwN3cyYUNleHJIYmdDN3ZQSHBuUUE1ZzJDSXo2c3hHL2ti?=
 =?utf-8?B?ZnhkN1RvMzJkSk5FRXlaTjFJeTFody8rdWVSdldWdVlJUlpqei9BREJQYnh2?=
 =?utf-8?B?RE52SE1aTkV0Z0V3TXlVc2wxZXIrejdWY1BoV0U0di9GVHVZZW9vdDMxTmhu?=
 =?utf-8?B?VmZVS0xPUHBMUkYzTEdsVU90MXRMNmtmRHdWN2V5RWVzdTdDSG5neStqUTJB?=
 =?utf-8?B?cHFHUzJDZCs5Yk44Um5ER1hTZEhQZmZuRWdJNGlBQnF3b2tvSk0xSnFoTDZk?=
 =?utf-8?B?VUJwTFJ6NU1uMlZvdDFjSTNWR0pwSDM2RTVCNFdScGZ0UTBaak56NnE4ZzVG?=
 =?utf-8?B?Sm12NUp4dUV6NFEwVmFCRUJYV1Z1S2ZDQlZROFA1eDNFYVpPSFhiQjA2NWF1?=
 =?utf-8?B?dmdGb1hJYjN4MmRPRlhYaHZGK0s4a3V5bmR5U2R3aW1pQXB2YkNMNXNIQVJO?=
 =?utf-8?B?aEpGMUQrU3o3VnVGOFVvTWJPWllITHpmN0JFUXJyYlgzSmY3VjhqQUZaaXBK?=
 =?utf-8?B?Ly9YQUhrQjRhaTBEdXJKZjNuK3RuMkw3N1BzUkVGRHhPdGQrQU9DeEdrc2Er?=
 =?utf-8?B?MCs0T1JULzNlSGVjSWp2MDNXQW9OTmZIdDc2Zm54WDljM0UydnhqRGFmSUV0?=
 =?utf-8?B?b3RvaFNzT0s3RXM0ZzJObjdVYVZZQ25ubytMdVNPQ1lTUWZzKzluNUwxYStz?=
 =?utf-8?B?UUdVS1VwUStOa0psTEg2djBscVRURlhRL3RyenEzbmJPbXVsQnFvdkVUeWFj?=
 =?utf-8?B?VFBDcnIwYnQrYkwwcXZlUWNwb2NVemxraHY0R2FJb0ZydDJaM0tHUDcxSDgy?=
 =?utf-8?B?RHljZERRbkJWWnlpazNkeHFKT1ZKRjB2YkNZZnp3VkV5UE1vYnVRdlErVEIz?=
 =?utf-8?B?MWt5OUJ6SGIvOW5XRWIycTE2aTNEdkc3RzE2Y2dqaVpIVFpjS2N4RW1sUUl1?=
 =?utf-8?B?S29qSWpiSy9HcEJhaTRQTHFRNy9EQVFyVE0wdFRnSUtjQmJYeS95a1hkdWVW?=
 =?utf-8?B?WFBJWnRrNzBneElTL2dsbDhmMWF3N3ZBTllHODdGZ2xxTWxLYXBSSDZOTDN4?=
 =?utf-8?B?VFpOVEtSODkrNGZUMkVzUHc1WHh4emV3dG1wdFkxZmlLUWp2UkdHV0w0R0Fs?=
 =?utf-8?B?cGxuaUZjdHdnY0w2Umx4enNyQ1I3b25XSnhVNkVBYVVPcWFvUllLT1lDQWta?=
 =?utf-8?Q?dw2i0RCODHH0KFmno702YUhrY?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5d057d02-202e-455c-2419-08dc3e3a177e
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2024 00:04:00.4855
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fMrNWJuZbpVvHtU5vULHra5nlOohMFppbMXMMbsrdGjiXjbcI2BNP38/AyPK2D3E4tINDNnsuGoV5bMHF0hfkg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8591
X-OriginatorOrg: intel.com


On 28/02/2024 3:41 pm, Sean Christopherson wrote:
> Allow mapping KVM's internal memslots used for EPT without unrestricted
> guest into L2, i.e. allow mapping the hidden TSS and the identity mapped
> page tables into L2.  Unlike the APIC access page, there is no correctness
> issue with letting L2 access the "hidden" memory.  Allowing these memslots
> to be mapped into L2 fixes a largely theoretical bug where KVM could
> incorrectly emulate subsequent _L1_ accesses as MMIO, and also ensures
> consistent KVM behavior for L2.
> 
> If KVM is using TDP, but L1 is using shadow paging for L2, then routing
> through kvm_handle_noslot_fault() will incorrectly cache the gfn as MMIO,
> and create an MMIO SPTE.  Creating an MMIO SPTE is ok, but only because
> kvm_mmu_page_role.guest_mode ensure KVM uses different roots for L1 vs.
> L2.  But vcpu->arch.mmio_gfn will remain valid, and could cause KVM to
> incorrectly treat an L1 access to the hidden TSS or identity mapped page
> tables as MMIO.
> 
> Furthermore, forcing L2 accesses to be treated as "no slot" faults doesn't
> actually prevent exposing KVM's internal memslots to L2, it simply forces
> KVM to emulate the access.  In most cases, that will trigger MMIO,
> amusingly due to filling vcpu->arch.mmio_gfn, but also because
> vcpu_is_mmio_gpa() unconditionally treats APIC accesses as MMIO, i.e. APIC
> accesses are ok.  But the hidden TSS and identity mapped page tables could
> go either way (MMIO or access the private memslot's backing memory).
> 
> Alternatively, the inconsistent emulator behavior could be addressed by
> forcing MMIO emulation for L2 access to all internal memslots, not just to
> the APIC.  But that's arguably less correct than letting L2 access the
> hidden TSS and identity mapped page tables, not to mention that it's
> *extremely* unlikely anyone cares what KVM does in this case.  From L1's
> perspective there is R/W memory at those memslots, the memory just happens
> to be initialized with non-zero data.  Making the memory disappear when it
> is accessed by L2 is far more magical and arbitrary than the memory
> existing in the first place.
> 
> The APIC access page is special because KVM _must_ emulate the access to
> do the right thing (emulate an APIC access instead of reading/writing the
> APIC access page).  And despite what commit 3a2936dedd20 ("kvm: mmu: Don't
> expose private memslots to L2") said, it's not just necessary when L1 is
> accelerating L2's virtual APIC, it's just as important (likely *more*
> imporant for correctness when L1 is passing through its own APIC to L
Reviewed-by: Kai Huang <kai.huang@intel.com>