start,
> + .end = end,
> + .handler = kvm_mmu_unmap_gfn_range,
> + .on_lock = kvm_mmu_invalidate_begin,
> + .on_unlock = (void *)kvm_null_fn,
> + .flush_on_ret = true,
> + .may_block = true,
> + };
> + struct kvm_mmu_noti
ej Szmigiero
> > Cc: Vlastimil Babka
> > Cc: David Hildenbrand
> > Cc: Quentin Perret
> > Cc: Michael Roth
> > Cc: Wang
> > Cc: Liam Merwick
> > Cc: Isaku Yamahata
> > Co-developed-by: Kirill A. Shutemov
> > Signed-off-by: Kirill A. S
);
> +
> + list_del(&gmem->entry);
> +
> + filemap_invalidate_unlock(inode->i_mapping);
> +
> + xa_destroy(&gmem->bindings);
> + kfree(gmem);
> +
> + kvm_put_kvm(kvm);
> +
> + return 0;
> +}
The lockdep complains with the filemapp
@ -809,6 +810,9 @@ struct kvm {
> >
> > #ifdef CONFIG_HAVE_KVM_PM_NOTIFIER
> > struct notifier_block pm_notifier;
> > +#endif
> > +#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> > + struct xarray mem_attr_array;
> > #endif
> > char stats_id[KVM_STATS_NAME_SIZE];
> > };
> > @@ -2301,4 +2305,14 @@ static inline void kvm_account_pgtable_pages(void
> > *virt, int nr)
> > /* Max number of entries allowed for each kvm dirty ring */
> > #define KVM_DIRTY_RING_MAX_ENTRIES 65536
> >
> > +#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> > +static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm,
> > gfn_t gfn)
> > +{
> > + return xa_to_value(xa_load(&kvm->mem_attr_array, gfn));
> > +}
> > +
> > +bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
> > +struct kvm_gfn_range *range);
>
> Used but no definition in this patch, it's defined in next patch 09.
> How about add weak version in this patch and let ARCHs to overide it ?
It is guarded by CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES.
--
Isaku Yamahata
to test it with legacy KVM MMU or shadow paging
> > to hit
> > the issue, though.
> >
>
> direct_pte_prefetch_many and prefetch_gpte both pass NULL for the
> fault parameter, so is_refcounted will evaluate to true. So the spte's
> refcounted bit will get set in that case.
Oops, my bad. My point is "unconditionally". Is the bit always set for
non-refcountable pages? Or non-refcountable pages are not prefeched?
--
Isaku Yamahata
counted page is prefetched? Or is it
> > possible in
> > practice?
>
> Prefetching is still done via gfn_to_page_many_atomic, which sets
> FOLL_GET. That's fixable, but it's not something this series currently
> does.
So if we prefetch a page, REFCOUNTED bit is cleared unconditionally with this
hunk. kvm_set_page_{dirty, accessed} won't be called as expected for prefetched
spte. If I read the patch correctly, REFCOUNTED bit in SPTE should represent
whether the corresponding page is ref-countable or not, right?
Because direct_pte_prefetch_many() is for legacy KVM MMU and FNAME(prefetch_pte)
is shadow paging, we need to test it with legacy KVM MMU or shadow paging to hit
the issue, though.
Thanks,
--
Isaku Yamahata
memtype_mask)
> spte |= static_call(kvm_x86_get_mt_mask)(vcpu, gfn,
--
Isaku Yamahata
vm_follow_pfn(&foll);
> +
> + if (!is_error_noslot_pfn(fault->pfn))
> + goto success;
> +
> + return RET_PF_CONTINUE;
> +success:
> + fault->hva = foll.hva;
> + fault->map_writable = foll.writable;
> return RET_PF_CONTINUE;
> }
>
> --
> 2.41.0.255.g8b1d071c50-goog
>
--
Isaku Yamahata
> + .atomic = true,
> + };
> + return __kvm_follow_pfn(&foll);
> }
> EXPORT_SYMBOL_GPL(gfn_to_pfn_memslot_atomic);
>
> diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
> index 180f1a09e6ba..ed896aee5396 100644
> --- a/virt/kvm/kvm_mm.h
> +++ b/virt/kvm/kvm_mm.h
> @@ -20,8 +20,7 @@
> #define KVM_MMU_UNLOCK(kvm) spin_unlock(&(kvm)->mmu_lock)
> #endif /* KVM_HAVE_MMU_RWLOCK */
>
> -kvm_pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool interruptible,
> - bool *async, bool write_fault, bool *writable);
> +kvm_pfn_t hva_to_pfn(struct kvm_follow_pfn *foll);
>
> #ifdef CONFIG_HAVE_KVM_PFNCACHE
> void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm,
> diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
> index 2d6aba677830..e3fefa753a51 100644
> --- a/virt/kvm/pfncache.c
> +++ b/virt/kvm/pfncache.c
> @@ -144,6 +144,12 @@ static kvm_pfn_t hva_to_pfn_retry(struct
> gfn_to_pfn_cache *gpc)
> kvm_pfn_t new_pfn = KVM_PFN_ERR_FAULT;
> void *new_khva = NULL;
> unsigned long mmu_seq;
> + struct kvm_follow_pfn foll = {
> + .slot = gpc->memslot,
> + .gfn = gpa_to_gfn(gpc->gpa),
> + .flags = FOLL_WRITE,
> + .hva = gpc->uhva,
> + };
>
> lockdep_assert_held(&gpc->refresh_lock);
>
> @@ -183,7 +189,7 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache
> *gpc)
> }
>
> /* We always request a writeable mapping */
> - new_pfn = hva_to_pfn(gpc->uhva, false, false, NULL, true, NULL);
> + new_pfn = hva_to_pfn(&foll);
> if (is_error_noslot_pfn(new_pfn))
> goto out_error;
>
> --
> 2.41.0.255.g8b1d071c50-goog
>
--
Isaku Yamahata
/*
> @@ -9390,8 +9382,6 @@ static int __kvm_x86_vendor_init(struct
> kvm_x86_init_ops *ops)
>* absolutely necessary, as most operations from this point forward
>* require unwinding.
>*/
> - kvm_ops_update(ops);
> -
> kvm_timer_init();
>
> if (pi_inject_timer == -1)
> @@ -9427,8 +9417,9 @@ static int __kvm_x86_vendor_init(struct
> kvm_x86_init_ops *ops)
> kvm_init_msr_list();
> return 0;
>
> -out_hardware_unsetup:
> - ops->runtime_ops->hardware_unsetup();
> +out_unwind_ops:
> + kvm_x86_ops.hardware_enable = NULL;
> + static_call(kvm_x86_hardware_unsetup)();
> out_mmu_exit:
> kvm_mmu_vendor_module_exit();
> out_free_percpu:
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
--
Isaku Yamahata
> +struct kvm_cpu_compat_check {
> + struct kvm_x86_init_ops *ops;
> + int *ret;
minor nitpick: just int ret. I don't see the necessity of the pointer.
Anyway overall it looks good to me.
Reviewed-by: Isaku Yamahata
> +};
> +
> +static int kvm_x86_check_processo
On Tue, Nov 08, 2022 at 01:09:27AM +,
"Huang, Kai" wrote:
> On Mon, 2022-11-07 at 13:46 -0800, Isaku Yamahata wrote:
> > > On Fri, Nov 04, 2022, Isaku Yamahata wrote:
> > > > Thanks for the patch series. I the rebased TDX KVM patch series and it
> &g
On Fri, Nov 04, 2022 at 08:27:14PM +,
Sean Christopherson wrote:
> On Fri, Nov 04, 2022, Isaku Yamahata wrote:
> > Thanks for the patch series. I the rebased TDX KVM patch series and it
> > worked.
> > Since cpu offline needs to be rejected in some cases(To keep at leas
On Thu, Nov 03, 2022 at 10:34:10PM +,
Sean Christopherson wrote:
> On Thu, Nov 03, 2022, Isaku Yamahata wrote:
> > On Wed, Nov 02, 2022 at 11:19:03PM +,
> > Sean Christopherson wrote:
> > > diff --git a/arch/x86/include/asm/kvm_host.h
> > > b/arch/x86/i
_usage_count) {
preempt_disable();
hardware_disable_nolock(NULL);
preempt_enable();
}
mutex_unlock(&kvm_lock);
- return 0;
+ return r;
}
static void hardware_disable_all_nolock(void)
--
Isaku Yamahata
, cpu is wrong. Although the function is called
in non-preemptive context, it's a bit confusing. So voting to remove it and
to use.
Thanks,
--
Isaku Yamahata
On Fri, Sep 23, 2022 at 04:58:41PM +1000,
Michael Ellerman wrote:
> isaku.yamah...@intel.com writes:
> > From: Isaku Yamahata
> >
> > Move processor compatibility check from kvm_arch_pr
From: Isaku Yamahata
Move processor compatibility check from kvm_arch_processor_compat() into
kvm_arch_hardware_setup(). The check does model name comparison with a
global variable, cur_cpu_spec. There is no point to check it at run time
on all processors.
kvmppc_core_check_processor_compat
à
> > https://aka.ms/LearnAboutSenderIdentification ]
> >
> > From: Isaku Yamahata
> >
> > Move processor compatibility check from kvm_arch_processor_compat() into
> > kvm_arch_hardware_setup(). The check does model name comparison with a
> > global variable, cur_cpu_spec.
From: Isaku Yamahata
Move processor compatibility check from kvm_arch_processor_compat() into
kvm_arch_hardware_setup(). The check does model name comparison with a
global variable, cur_cpu_spec. There is no point to check it at run time
on all processors.
Suggested-by: Sean Christopherson
20 matches
Mail list logo
