Re: [PATCH 0/2] x86/kvm: Force legacy PCI hole as WB under SNP/TDX

[Sean Christopherson]

On Thu, Aug 21, 2025, Binbin Wu wrote:
> On 8/21/2025 11:30 AM, Binbin Wu wrote:
> > Variable MTRR has requirement for range size and alignment:
> > For ranges greater than 4 KBytes, each range must be of length 2^n and its base
> > address must be aligned on a 2^n boundary, where n is a value equal to or
> > greater than 12. The base-address alignment value cannot be less than its length.
> 
> Wait, Linux kernel converts MTRR register values to MTRR state (base and size) and
> cache it for later lookups (refer to map_add_var()). I.e., in Linux kernel,
> only the cached state will be used.
> 
> These MTRR register values are never programmed when using
> guest_force_mtrr_state() , so even the values doesn't meet the requirement
> from hardware perspective, Linux kernel can still get the right base and
> size.

Yeah.  I forget what happens if the ranges don't meet the power-of-2 requirements,
but the mask+match logic should work jus tfine.

> No bothering to force the base and size alignment.
> But a comment would be helpful.
> Also, BIT(11) could be replaced by MTRR_PHYSMASK_V.

Ha!  I spent a good 5 minutes looking for a #define couldn't find one.  What a
bizarre name...

> How about:
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 90097df4eafd..a9582ffc3088 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -934,9 +934,15 @@ static void kvm_sev_hc_page_enc_status(unsigned long pfn, int npages, bool enc)
>  static void __init kvm_init_platform(void)
>  {
>         u64 tolud = e820__end_of_low_ram_pfn() << PAGE_SHIFT;
> +       /*
> +        * The range's base address and size may not meet the alignment
> +        * requirement for variable MTRR. However, Linux guest never
> +        * programs MTRRs when forcing guest MTRR state, no bothering to
> +        * enforce the base and range size alignment.
> +        */
>         struct mtrr_var_range pci_hole = {
>                 .base_lo = tolud | X86_MEMTYPE_UC,
> -               .mask_lo = (u32)(~(SZ_4G - tolud - 1)) | BIT(11),
> +               .mask_lo = (u32)(~(SZ_4G - tolud - 1)) | MTRR_PHYSMASK_V,
>                 .mask_hi = (BIT_ULL(boot_cpu_data.x86_phys_bits) - 1) >> 32,
>         };
> 
> 
> I tested it in my setup, it can fix the issue of TPM driver failure with the
> modified ACPI table for TPM in QEMU.
> 
> 
> Hi Vishal,
> Could you test it with google's VMM?

Vishal is OOO for a few days.  I pinged our internal bug tracker, I'll find
someone to test.

Thanks much!