[PATCH 5/8] xen/arch: Strip out tlb-clock stubs for non-implementors

Andrew Cooper posted 8 patches 9 months, 1 week ago
[PATCH 5/8] xen/arch: Strip out tlb-clock stubs for non-implementors
Posted by Andrew Cooper 9 months, 1 week ago
Now that there's a common stub implementation TLB clocks, there's no need for
architectures to provide their own.

Repeatedly zeroing page->tlbflush_timestamp is no use, so provide an even more
empty common stub for page_set_tlbflush_timestamp().

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>
---
 xen/arch/arm/include/asm/flushtlb.h   | 14 --------------
 xen/arch/ppc/include/asm/flushtlb.h   | 14 --------------
 xen/arch/riscv/include/asm/flushtlb.h | 14 --------------
 xen/include/xen/tlb-clock.h           |  1 +
 4 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/xen/arch/arm/include/asm/flushtlb.h b/xen/arch/arm/include/asm/flushtlb.h
index e45fb6d97b02..6f69a5bdc8c2 100644
--- a/xen/arch/arm/include/asm/flushtlb.h
+++ b/xen/arch/arm/include/asm/flushtlb.h
@@ -3,20 +3,6 @@
 
 #include <xen/cpumask.h>
 
-/*
- * Filter the given set of CPUs, removing those that definitely flushed their
- * TLB since @page_timestamp.
- */
-/* XXX lazy implementation just doesn't clear anything.... */
-static inline void tlbflush_filter(cpumask_t *mask, uint32_t page_timestamp) {}
-
-#define tlbflush_current_time()                 (0)
-
-static inline void page_set_tlbflush_timestamp(struct page_info *page)
-{
-    page->tlbflush_timestamp = tlbflush_current_time();
-}
-
 #if defined(CONFIG_ARM_32)
 # include <asm/arm32/flushtlb.h>
 #elif defined(CONFIG_ARM_64)
diff --git a/xen/arch/ppc/include/asm/flushtlb.h b/xen/arch/ppc/include/asm/flushtlb.h
index afcb74078368..f89037bd4543 100644
--- a/xen/arch/ppc/include/asm/flushtlb.h
+++ b/xen/arch/ppc/include/asm/flushtlb.h
@@ -4,20 +4,6 @@
 
 #include <xen/cpumask.h>
 
-/*
- * Filter the given set of CPUs, removing those that definitely flushed their
- * TLB since @page_timestamp.
- */
-/* XXX lazy implementation just doesn't clear anything.... */
-static inline void tlbflush_filter(cpumask_t *mask, uint32_t page_timestamp) {}
-
-#define tlbflush_current_time()                 (0)
-
-static inline void page_set_tlbflush_timestamp(struct page_info *page)
-{
-    page->tlbflush_timestamp = tlbflush_current_time();
-}
-
 /* Flush specified CPUs' TLBs */
 void arch_flush_tlb_mask(const cpumask_t *mask);
 
diff --git a/xen/arch/riscv/include/asm/flushtlb.h b/xen/arch/riscv/include/asm/flushtlb.h
index 51c8f753c51e..23739a22fb2a 100644
--- a/xen/arch/riscv/include/asm/flushtlb.h
+++ b/xen/arch/riscv/include/asm/flushtlb.h
@@ -20,20 +20,6 @@ static inline void flush_tlb_range_va(vaddr_t va, size_t size)
     sbi_remote_sfence_vma(NULL, va, size);
 }
 
-/*
- * Filter the given set of CPUs, removing those that definitely flushed their
- * TLB since @page_timestamp.
- */
-/* XXX lazy implementation just doesn't clear anything.... */
-static inline void tlbflush_filter(cpumask_t *mask, uint32_t page_timestamp) {}
-
-#define tlbflush_current_time() (0)
-
-static inline void page_set_tlbflush_timestamp(struct page_info *page)
-{
-    BUG_ON("unimplemented");
-}
-
 /* Flush specified CPUs' TLBs */
 void arch_flush_tlb_mask(const cpumask_t *mask);
 
diff --git a/xen/include/xen/tlb-clock.h b/xen/include/xen/tlb-clock.h
index 796c0be7fbef..467f6d64a6ca 100644
--- a/xen/include/xen/tlb-clock.h
+++ b/xen/include/xen/tlb-clock.h
@@ -44,6 +44,7 @@ static inline void accumulate_tlbflush(
     bool *need_tlbflush, const struct page_info *page,
     uint32_t *tlbflush_timestamp) {}
 static inline void filtered_flush_tlb_mask(uint32_t tlbflush_timestamp) {}
+static inline void page_set_tlbflush_timestamp(struct page_info *page) {}
 
 #endif /* !CONFIG_HAS_TLB_CLOCK*/
 #endif /* XEN_TLB_CLOCK_H */
-- 
2.39.5


Re: [PATCH 5/8] xen/arch: Strip out tlb-clock stubs for non-implementors
Posted by Jan Beulich 9 months, 1 week ago
On 12.03.2025 18:45, Andrew Cooper wrote:
> Now that there's a common stub implementation TLB clocks, there's no need for
> architectures to provide their own.
> 
> Repeatedly zeroing page->tlbflush_timestamp is no use, so provide an even more
> empty common stub for page_set_tlbflush_timestamp().

At which point the field itself could in principle go away. There are three
printk()s (accompanying BUG()s) which use it; surely we can find a way to
abstract that out. This may then still be enough of a reason to introduce
HAS_TLB_CLOCK.

Jan
Re: [PATCH 5/8] xen/arch: Strip out tlb-clock stubs for non-implementors
Posted by Andrew Cooper 9 months, 1 week ago
On 13/03/2025 1:05 pm, Jan Beulich wrote:
> On 12.03.2025 18:45, Andrew Cooper wrote:
>> Now that there's a common stub implementation TLB clocks, there's no need for
>> architectures to provide their own.
>>
>> Repeatedly zeroing page->tlbflush_timestamp is no use, so provide an even more
>> empty common stub for page_set_tlbflush_timestamp().
> At which point the field itself could in principle go away. There are three
> printk()s (accompanying BUG()s) which use it; surely we can find a way to
> abstract that out. This may then still be enough of a reason to introduce
> HAS_TLB_CLOCK.

I wanted to remove the field, but it wasn't trivial, and I've probably
spent more time than I can afford on this.

I'm tempted to leave a TODO in tlb-clock.h to make it clear that there's
more that ought to be done.

~Andrew
Re: [PATCH 5/8] xen/arch: Strip out tlb-clock stubs for non-implementors
Posted by Jan Beulich 9 months, 1 week ago
On 13.03.2025 15:11, Andrew Cooper wrote:
> On 13/03/2025 1:05 pm, Jan Beulich wrote:
>> On 12.03.2025 18:45, Andrew Cooper wrote:
>>> Now that there's a common stub implementation TLB clocks, there's no need for
>>> architectures to provide their own.
>>>
>>> Repeatedly zeroing page->tlbflush_timestamp is no use, so provide an even more
>>> empty common stub for page_set_tlbflush_timestamp().
>> At which point the field itself could in principle go away. There are three
>> printk()s (accompanying BUG()s) which use it; surely we can find a way to
>> abstract that out. This may then still be enough of a reason to introduce
>> HAS_TLB_CLOCK.
> 
> I wanted to remove the field, but it wasn't trivial, and I've probably
> spent more time than I can afford on this.

I can understand this. It'll remain to be seen how useful HAS_TLB_CLOCK is
with patch 4 corrected. And of course it's ...

> I'm tempted to leave a TODO in tlb-clock.h to make it clear that there's
> more that ought to be done.

... kind of okay to leave parts to be done later, as long as it's at least
halfway clear what it is that wants doing.

Jan