lkml.org 
[lkml]   [2023]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v11 058/113] KVM: TDX: MTRR: implement get_mt_mask() for TDX
    On Tue, Jan 17, 2023 at 03:11:46AM +0000,
    "Huang, Kai" <kai.huang@intel.com> wrote:

    > On Thu, 2023-01-12 at 08:32 -0800, isaku.yamahata@intel.com wrote:
    > > From: Isaku Yamahata <isaku.yamahata@intel.com>
    > >
    > > Although TDX supports only WB for private GPA, MTRR/PAT for shared GPA
    > > should be supported. Implement get_mt_mask() following vmx case.
    >
    > By far this is the first patch to handle MTRR/PAT. There's absolutely no
    > background have been explained.
    >
    > So what about MTRR/PAT related MSRs handling? No code needed to handle?
    >
    > I was expecting there should be at least some words here to explain how TDX
    > handles them, and if no handling is required in KVM, why.
    >
    > W/o those, I don't think this patch is reviewable.

    I've updated the commit message.


    > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
    > > ---
    > > arch/x86/kvm/vmx/main.c | 10 +++++++++-
    > > arch/x86/kvm/vmx/tdx.c | 19 +++++++++++++++++++
    > > arch/x86/kvm/vmx/x86_ops.h | 2 ++
    > > 3 files changed, 30 insertions(+), 1 deletion(-)
    > >
    > > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
    > > index 770d1b29d1c3..4319f6d7a4da 100644
    > > --- a/arch/x86/kvm/vmx/main.c
    > > +++ b/arch/x86/kvm/vmx/main.c
    > > @@ -158,6 +158,14 @@ static void vt_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
    > > vmx_load_mmu_pgd(vcpu, root_hpa, pgd_level);
    > > }
    > >
    > > +static u8 vt_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
    > > +{
    > > + if (is_td_vcpu(vcpu))
    > > + return tdx_get_mt_mask(vcpu, gfn, is_mmio);
    > > +
    > > + return vmx_get_mt_mask(vcpu, gfn, is_mmio);
    > > +}
    > > +
    > > static int vt_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
    > > {
    > > if (!is_td(kvm))
    > > @@ -267,7 +275,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
    > >
    > > .set_tss_addr = vmx_set_tss_addr,
    > > .set_identity_map_addr = vmx_set_identity_map_addr,
    > > - .get_mt_mask = vmx_get_mt_mask,
    > > + .get_mt_mask = vt_get_mt_mask,
    > >
    > > .get_exit_info = vmx_get_exit_info,
    > >
    > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
    > > index e68816999387..c4c5a8f786c1 100644
    > > --- a/arch/x86/kvm/vmx/tdx.c
    > > +++ b/arch/x86/kvm/vmx/tdx.c
    > > @@ -309,6 +309,25 @@ int tdx_vm_init(struct kvm *kvm)
    > > return 0;
    > > }
    > >
    > > +u8 tdx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
    > > +{
    > > + /* TDX private GPA is always WB. */
    > > + if (gfn & kvm_gfn_shared_mask(vcpu->kvm)) {
    >
    > First of all, private GPA doesn't have 'shared bit' set, so comment doesn't
    > reflect code.
    >
    > Secondly (and again), IIUC the shared bit of the gfn has been stripped out long
    > time ago, so this is incorrect.
    >
    > Please don't sliently ignore other people's comment:
    >
    > https://lore.kernel.org/lkml/Y19NzlQcwhV%2F2wl3@debian.me/T/#mf319d5b718519709362f9f094bfc5b53fd870241

    Make the logic common for vmx and tdx. the difference is to check cr0.cd.

    Thanks,
    --
    Isaku Yamahata <isaku.yamahata@gmail.com>

    \
     
     \ /
      Last update: 2023-03-27 00:37    [W:4.296 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site