linux-kernel - Re: [PATCH v2 06/10] KVM: x86: Implement kvm_arch_vcpu_map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <75b213fd73fcb5872703f89a9c6bb67ea91e3bd7.camel@intel.com>
Date: Tue, 16 Apr 2024 15:12:34 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>
CC: "seanjc@...gle.com" <seanjc@...gle.com>, "Huang, Kai"
	<kai.huang@...el.com>, "federico.parola@...ito.it"
	<federico.parola@...ito.it>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "isaku.yamahata@...il.com"
	<isaku.yamahata@...il.com>, "dmatlack@...gle.com" <dmatlack@...gle.com>,
	"michael.roth@....com" <michael.roth@....com>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>
Subject: Re: [PATCH v2 06/10] KVM: x86: Implement kvm_arch_vcpu_map_memory()

On Wed, 2024-04-10 at 15:07 -0700, isaku.yamahata@...el.com wrote:
> From: Isaku Yamahata <isaku.yamahata@...el.com>
> 
> Wire KVM_MAP_MEMORY ioctl to kvm_mmu_map_tdp_page() to populate guest
> memory.  When KVM_CREATE_VCPU creates vCPU, it initializes the x86
> KVM MMU part by kvm_mmu_create() and kvm_init_mmu().  vCPU is ready to
> invoke the KVM page fault handler.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
> ---
> v2:
> - Catch up the change of struct kvm_memory_mapping. (Sean)
> - Removed mapping level check. Push it down into vendor code. (David, Sean)
> - Rename goal_level to level. (Sean)
> - Drop kvm_arch_pre_vcpu_map_memory(), directly call kvm_mmu_reload().
>   (David, Sean)
> - Fixed the update of mapping.
> ---
>  arch/x86/kvm/x86.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2d2619d3eee4..2c765de3531e 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4713,6 +4713,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long
> ext)
>         case KVM_CAP_VM_DISABLE_NX_HUGE_PAGES:
>         case KVM_CAP_IRQFD_RESAMPLE:
>         case KVM_CAP_MEMORY_FAULT_INFO:
> +       case KVM_CAP_MAP_MEMORY:
>                 r = 1;
>                 break;

Should we add this after all of the pieces are in place?

>         case KVM_CAP_EXIT_HYPERCALL:
> @@ -5867,6 +5868,35 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu
> *vcpu,
>         }
>  }
>  
> +int kvm_arch_vcpu_map_memory(struct kvm_vcpu *vcpu,
> +                            struct kvm_memory_mapping *mapping)
> +{
> +       u64 end, error_code = 0;
> +       u8 level = PG_LEVEL_4K;
> +       int r;
> +
> +       /*
> +        * Shadow paging uses GVA for kvm page fault.  The first
> implementation
> +        * supports GPA only to avoid confusion.
> +        */
> +       if (!tdp_enabled)
> +               return -EOPNOTSUPP;

It's not confusion, it's that you can't pre-map GPAs for legacy shadow paging.
Or you are saying why not to support pre-mapping GVAs? I think that discussion
belongs more in the commit log. The code should just say it's not possible to
pre-map GPAs in shadow paging.

> +
> +       /* reload is optimized for repeated call. */
> +       kvm_mmu_reload(vcpu);
> +
> +       r = kvm_tdp_map_page(vcpu, mapping->base_address, error_code, &level);
> +       if (r)
> +               return r;
> +
> +       /* mapping->base_address is not necessarily aligned to level-hugepage.
> */
> +       end = (mapping->base_address & KVM_HPAGE_MASK(level)) +
> +               KVM_HPAGE_SIZE(level);
> +       mapping->size -= end - mapping->base_address;
> +       mapping->base_address = end;
> +       return r;
> +}
> +
>  long kvm_arch_vcpu_ioctl(struct file *filp,
>                          unsigned int ioctl, unsigned long arg)
>  {