Re: [PATCH -next] memregion: Add arch_flush_memregion() interface
Andrew Morton wrote: > I really dislike the term "flush". Sometimes it means writeback, > sometimes it means invalidate. Perhaps at other times it means > both. > > Can we please be very clear in comments and changelogs about exactly > what this "flush" does. With bonus points for being more specific in the > function naming? > That's a good point, "flush" has been cargo-culted along in Linux's cache management APIs to mean write-back-and-invalidate. In this case I think this API is purely about invalidate. It just so happens that x86 has not historically had a global invalidate instruction readily available which leads to the overuse of wbinvd. It would be nice to make clear that this API is purely about invalidating any data cached for a physical address impacted by address space management event (secure erase / new region provision). Write-back is an unnecessary side-effect. So how about: s/arch_flush_memregion/cpu_cache_invalidate_memregion/?
Re: [PATCH -next] memregion: Add arch_flush_memregion() interface
I really dislike the term "flush". Sometimes it means writeback, sometimes it means invalidate. Perhaps at other times it means both. Can we please be very clear in comments and changelogs about exactly what this "flush" does. With bonus points for being more specific in the function naming?
Re: [PATCH -next] memregion: Add arch_flush_memregion() interface
Not sure the proper way to route this (akpm?). But unless any remaining objections, could this be picked up? Thanks, Davidlohr
Re: [PATCH -next] memregion: Add arch_flush_memregion() interface
On Wed, 07 Sep 2022, Borislav Petkov wrote:
On Mon, Aug 29, 2022 at 02:29:18PM -0700, Davidlohr Bueso wrote:
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 1abd5438f126..18463cb704fb 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -330,6 +330,20 @@ void arch_invalidate_pmem(void *addr, size_t size)
EXPORT_SYMBOL_GPL(arch_invalidate_pmem);
#endif
+#ifdef CONFIG_ARCH_HAS_MEMREGION_INVALIDATE
+bool arch_has_flush_memregion(void)
+{
+ return !cpu_feature_enabled(X86_FEATURE_HYPERVISOR);
This looks really weird. Why does this need to care about HV at all?
So the context here is:
e2efb6359e62 ("ACPICA: Avoid cache flush inside virtual machines")
Does that nfit stuff even run in guests?
No, nor does cxl. This was mostly in general a precautionary check such
that the api is unavailable in VMs.
+EXPORT_SYMBOL(arch_has_flush_memregion);
...
+EXPORT_SYMBOL(arch_flush_memregion);
Why aren't those exports _GPL?
Fine by me.
Thanks,
Davidlohr
Re: [PATCH -next] memregion: Add arch_flush_memregion() interface
On Wed, 07 Sep 2022, Dan Williams wrote:
Davidlohr Bueso wrote:
On Wed, 07 Sep 2022, Borislav Petkov wrote:
>On Mon, Aug 29, 2022 at 02:29:18PM -0700, Davidlohr Bueso wrote:
>> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
>> index 1abd5438f126..18463cb704fb 100644
>> --- a/arch/x86/mm/pat/set_memory.c
>> +++ b/arch/x86/mm/pat/set_memory.c
>> @@ -330,6 +330,20 @@ void arch_invalidate_pmem(void *addr, size_t size)
>> EXPORT_SYMBOL_GPL(arch_invalidate_pmem);
>> #endif
>>
>> +#ifdef CONFIG_ARCH_HAS_MEMREGION_INVALIDATE
>> +bool arch_has_flush_memregion(void)
>> +{
>> + return !cpu_feature_enabled(X86_FEATURE_HYPERVISOR);
>
>This looks really weird. Why does this need to care about HV at all?
So the context here is:
e2efb6359e62 ("ACPICA: Avoid cache flush inside virtual machines")
>
>Does that nfit stuff even run in guests?
No, nor does cxl. This was mostly in general a precautionary check such
that the api is unavailable in VMs.
To be clear nfit stuff and CXL does run in guests, but they do not
support secure-erase in a guest.
Yes, I meant the feats this api enables.
However, the QEMU CXL enabling is building the ability to do *guest
physical* address space management, but in that case the driver can be
paravirtualized to realize that it is not managing host-physical address
space and does not need to flush caches. That will need some indicator
to differentiate virtual CXL memory expanders from assigned devices. Is
there such a thing as a PCIe-virtio extended capability to differentiate
physical vs emulated devices?
In any case such check would be specific to each user (cxl in this case),
and outside the scope of _this_ particular api. Here we just really want to
avoid the broken TDX guest bits.
Thanks,
Davidlohr
Re: [PATCH -next] memregion: Add arch_flush_memregion() interface
Davidlohr Bueso wrote:
> On Wed, 07 Sep 2022, Borislav Petkov wrote:
>
> >On Mon, Aug 29, 2022 at 02:29:18PM -0700, Davidlohr Bueso wrote:
> >> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> >> index 1abd5438f126..18463cb704fb 100644
> >> --- a/arch/x86/mm/pat/set_memory.c
> >> +++ b/arch/x86/mm/pat/set_memory.c
> >> @@ -330,6 +330,20 @@ void arch_invalidate_pmem(void *addr, size_t size)
> >> EXPORT_SYMBOL_GPL(arch_invalidate_pmem);
> >> #endif
> >>
> >> +#ifdef CONFIG_ARCH_HAS_MEMREGION_INVALIDATE
> >> +bool arch_has_flush_memregion(void)
> >> +{
> >> + return !cpu_feature_enabled(X86_FEATURE_HYPERVISOR);
> >
> >This looks really weird. Why does this need to care about HV at all?
>
> So the context here is:
>
> e2efb6359e62 ("ACPICA: Avoid cache flush inside virtual machines")
>
> >
> >Does that nfit stuff even run in guests?
>
> No, nor does cxl. This was mostly in general a precautionary check such
> that the api is unavailable in VMs.
To be clear nfit stuff and CXL does run in guests, but they do not
support secure-erase in a guest.
However, the QEMU CXL enabling is building the ability to do *guest
physical* address space management, but in that case the driver can be
paravirtualized to realize that it is not managing host-physical address
space and does not need to flush caches. That will need some indicator
to differentiate virtual CXL memory expanders from assigned devices. Is
there such a thing as a PCIe-virtio extended capability to differentiate
physical vs emulated devices?
Re: [PATCH -next] memregion: Add arch_flush_memregion() interface
Davidlohr Bueso wrote: > Not sure the proper way to route this (akpm?). But unless any remaining > objections, could this be picked up? My plan was, barring objections, to take it through the CXL tree with its first user, the CXL security commands.
Re: [PATCH v8 0/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind
ping 在 2022/9/2 18:35, Shiyang Ruan 写道: Changes since v7: 1. Add P1 to fix calculation mistake 2. Add P2 to move drop_pagecache_sb() to super.c for xfs to use 3. P3: Add invalidate all mappings after sync. 4. P3: Set offset to be start of device when it is to be removed. 5. Rebase on 6.0-rc3 + Darrick's patch[1] + Dan's patch[2]. Changes since v6: 1. Rebase on 6.0-rc2 and Darrick's patch[1]. [1]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/ [2]: https://lore.kernel.org/linux-xfs/166153426798.2758201.15108211981034512993.st...@dwillia2-xfh.jf.intel.com/ Shiyang Ruan (3): xfs: fix the calculation of length and end fs: move drop_pagecache_sb() for others to use mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind drivers/dax/super.c | 3 ++- fs/drop_caches.c| 33 - fs/super.c | 34 ++ fs/xfs/xfs_notify_failure.c | 31 +++ include/linux/fs.h | 1 + include/linux/mm.h | 1 + 6 files changed, 65 insertions(+), 38 deletions(-)
