On Thu, May 26, 2022 at 10:48:56AM +0800, Xiaoyao Li wrote: > On 5/24/2022 3:08 PM, Gerd Hoffmann wrote: > > On Thu, May 12, 2022 at 11:17:45AM +0800, Xiaoyao Li wrote: > > > TDX guest cannot go to real mode, so just skip the setup of isa-bios. > > > > Does isa-bios setup cause any actual problems? > > (same question for patch #19). > > It causes mem_region split and mem_slot deletion on KVM. > > TDVF marks pages starting from 0x800000 as TEMP_MEM and TD_HOB, which are > TD's private memory and are TDH_MEM_PAGE_ADD'ed to TD via > KVM_TDX_INIT_MEM_REGION > > However, if isa-bios and pc.rom are not skipped, the memory_region > initialization of them is after KVM_TDX_INIT_MEM_REGION in > tdx_machine_done_notify(). (I didn't figure out why this order though) > > And the it causes memory region split that splits > [0, ram_below_4g) > to > [0, 0xc0 000), > [0xc0 000, 0xe0 000), > [0xe0 000, 0x100 000), > [0x100 000, ram_below_4g) > > which causes mem_slot deletion on KVM. On KVM side, we lose the page content > when mem_slot deletion. Thus, the we lose the content of TD HOB. Hmm, removing and re-creating memory slots shouldn't cause page content go away. I'm wondering what the *real* problem is? Maybe you loose tdx-specific state, i.e. this removes TDH_MEM_PAGE_ADD changes? > Yes, the better solution seems to be ensure KVM_TDX_INIT_MEM_REGION is > called after all the mem region is settled down. Yes, especially if tdx can't tolerate memory slots coming and going. > But I haven't figured out the reason why the isa-bios and pc.rom > initialization happens after machine_init_done_notifier Probably happens when a flatview is created from the address space. Maybe that is delayed somehow for machine creation, so all the address space updates caused by device creation don't lead to lots of flatviews being created and thrown away. > on the other hand, to keep isa-bios and pc.rom, we need additional work to > copy the content from the end_of_4G to end_of_1M. There is no need for copying, end_of_1M is a alias memory region for end_of_4G, so the backing storage is the same. > I'm not sure if isa-bios and pc.rom are needed from people on TD guest, so I > just skip them for simplicity, Given that TDX guests start in 32bit mode not in real mode everything should work fine without isa-bios. I'd prefer to avoid creating a special case for tdx though. Should make long-term maintenance a bit easier when this is not needed. take care, Gerd