On Wed, Mar 20, 2024 at 03:39:44AM -0500, Michael Roth <michael.roth@xxxxxxx> wrote: > TODO: make this SNP-specific if TDX disables legacy ROMs in general TDX disables pc.rom, not disable isa-bios. IIRC, TDX doesn't need pc pflash. Xiaoyao can chime in. Thanks, > > Current SNP guest kernels will attempt to access these regions with > with C-bit set, so guest_memfd is needed to handle that. Otherwise, > kvm_convert_memory() will fail when the guest kernel tries to access it > and QEMU attempts to call KVM_SET_MEMORY_ATTRIBUTES to set these ranges > to private. > > Whether guests should actually try to access ROM regions in this way (or > need to deal with legacy ROM regions at all), is a separate issue to be > addressed on kernel side, but current SNP guest kernels will exhibit > this behavior and so this handling is needed to allow QEMU to continue > running existing SNP guest kernels. > > Signed-off-by: Michael Roth <michael.roth@xxxxxxx> > --- > hw/i386/pc.c | 13 +++++++++---- > hw/i386/pc_sysfw.c | 13 ++++++++++--- > 2 files changed, 19 insertions(+), 7 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index feb7a93083..5feaeb43ee 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1011,10 +1011,15 @@ void pc_memory_init(PCMachineState *pcms, > pc_system_firmware_init(pcms, rom_memory); > > option_rom_mr = g_malloc(sizeof(*option_rom_mr)); > - memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE, > - &error_fatal); > - if (pcmc->pci_enabled) { > - memory_region_set_readonly(option_rom_mr, true); > + if (machine_require_guest_memfd(machine)) { > + memory_region_init_ram_guest_memfd(option_rom_mr, NULL, "pc.rom", > + PC_ROM_SIZE, &error_fatal); > + } else { > + memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE, > + &error_fatal); > + if (pcmc->pci_enabled) { > + memory_region_set_readonly(option_rom_mr, true); > + } > } > memory_region_add_subregion_overlap(rom_memory, > PC_ROM_MIN_VGA, > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c > index 9dbb3f7337..850f86edd4 100644 > --- a/hw/i386/pc_sysfw.c > +++ b/hw/i386/pc_sysfw.c > @@ -54,8 +54,13 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory, > /* map the last 128KB of the BIOS in ISA space */ > isa_bios_size = MIN(flash_size, 128 * KiB); > isa_bios = g_malloc(sizeof(*isa_bios)); > - memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size, > - &error_fatal); > + if (machine_require_guest_memfd(current_machine)) { > + memory_region_init_ram_guest_memfd(isa_bios, NULL, "isa-bios", > + isa_bios_size, &error_fatal); > + } else { > + memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size, > + &error_fatal); > + } > memory_region_add_subregion_overlap(rom_memory, > 0x100000 - isa_bios_size, > isa_bios, > @@ -68,7 +73,9 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory, > ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size), > isa_bios_size); > > - memory_region_set_readonly(isa_bios, true); > + if (!machine_require_guest_memfd(current_machine)) { > + memory_region_set_readonly(isa_bios, true); > + } > } > > static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms, > -- > 2.25.1 > > -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>